From 698b0c35f1451a93f18bc2618075ee301b454816 Mon Sep 17 00:00:00 2001 From: Adarsh <122873385+Adarsh-Me@users.noreply.github.com> Date: Thu, 3 Sep 2026 21:17:54 +0530 Subject: [PATCH 1/2] fix(docker): make auth bootstrap safe for mounted and upgraded configs The entrypoint's grep/sed property rewriting disagrees with HugeConfig on mounted or upgraded configs: escaped keys, ':'/whitespace separators, line continuations, and duplicate definitions are all read differently, so a mounted config could end up with two logical definitions of one key. Property reading/writing now goes through props.awk, which implements the java.util.Properties grammar (comments, both separators, continuations, backslash escapes, first-definition-wins duplicates) and keeps every untouched line byte-for-byte. Values travel through environment variables instead of command arguments, so a PASSWORD no longer shows up in 'ps' output when a key is rewritten in place. enable-auth.sh appended authentication definitions whenever conf-bak/ was absent, which on a mounted config created duplicate definitions that the properties parser (first definition wins) and the yaml parser (last definition wins) resolved in opposite directions -- Gremlin and REST could land on different authenticators with no error from either. Its appends are now guarded per file, only an absent or still commented-out definition triggers an append, re-runs are idempotent, and the authenticator class is overridable through AUTHENTICATOR_CLASS. The entrypoint aligns both sides before calling it: it copies a yaml authenticator into rest-server.properties, or exports the REST one for the yaml append, and warns without touching anything when the two name genuinely different authenticators. The unit test suite covers escaped keys, continuations, get-mode semantics, and comment-guarded appends; the entrypoint harness now ships props.awk into its sandbox, and both server Dockerfiles COPY it next to the entrypoint. Fixes #3133 --- hugegraph-server/Dockerfile | 1 + hugegraph-server/Dockerfile-hstore | 1 + .../docker/docker-entrypoint-test.sh | 1 + .../docker/docker-entrypoint.sh | 83 +++++-- .../hugegraph-dist/docker/props.awk | 227 ++++++++++++++++++ .../docker/test/test-docker-entrypoint.sh | 57 ++++- .../src/assembly/static/bin/enable-auth.sh | 26 +- 7 files changed, 375 insertions(+), 21 deletions(-) create mode 100644 hugegraph-server/hugegraph-dist/docker/props.awk diff --git a/hugegraph-server/Dockerfile b/hugegraph-server/Dockerfile index 44bc9aa515..f360adcb68 100644 --- a/hugegraph-server/Dockerfile +++ b/hugegraph-server/Dockerfile @@ -66,6 +66,7 @@ RUN apt-get -q update \ COPY hugegraph-server/hugegraph-dist/docker/scripts/remote-connect.groovy ./scripts COPY hugegraph-server/hugegraph-dist/docker/scripts/detect-storage.groovy ./scripts COPY hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh . +COPY hugegraph-server/hugegraph-dist/docker/props.awk . RUN chmod 755 ./docker-entrypoint.sh EXPOSE 8080 diff --git a/hugegraph-server/Dockerfile-hstore b/hugegraph-server/Dockerfile-hstore index fc99034728..81f1063d90 100644 --- a/hugegraph-server/Dockerfile-hstore +++ b/hugegraph-server/Dockerfile-hstore @@ -68,6 +68,7 @@ RUN apt-get -q update \ COPY hugegraph-server/hugegraph-dist/docker/scripts/remote-connect.groovy ./scripts #COPY hugegraph-server/hugegraph-dist/docker/scripts/detect-storage.groovy ./scripts COPY hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh . +COPY hugegraph-server/hugegraph-dist/docker/props.awk . RUN chmod 755 ./docker-entrypoint.sh EXPOSE 8080 diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh index 6e22885ebe..6250ab4f14 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh @@ -23,6 +23,7 @@ trap 'rm -rf "${TEST_HOME}"' EXIT mkdir -p "${TEST_HOME}/bin" "${TEST_HOME}/conf/graphs" "${TEST_HOME}/docker" cp "${SCRIPT_DIR}/docker-entrypoint.sh" "${TEST_HOME}/docker-entrypoint.sh" +cp "${SCRIPT_DIR}/props.awk" "${TEST_HOME}/props.awk" touch "${TEST_HOME}/docker/init_complete" cat > "${TEST_HOME}/conf/rest-server.properties" <<'EOF' diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh index fe9974c430..ee2994776c 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh @@ -26,6 +26,18 @@ mkdir -p "${DOCKER_FOLDER}" log() { echo "[hugegraph-server-entrypoint] $*"; } +# Property reading/writing goes through props.awk, which implements the +# java.util.Properties grammar HugeConfig applies (escapes, `:`/whitespace +# separators, continuations, first-definition-wins duplicates). grep/sed +# rewrites disagree with it on mounted or upgraded configs, silently +# producing two definitions of one key. Values move through environment +# variables rather than argv so a PASSWORD never shows up in `ps` output. +PROPS_AWK="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/props.awk" +if [[ ! -f "${PROPS_AWK}" ]]; then + log "ERROR: props.awk not found next to the entrypoint" + exit 1 +fi + encode_prop_value() { local value="$1" encoded="" char local i @@ -48,18 +60,10 @@ encode_prop_value() { set_prop_encoded() { local key="$1" encoded_val="$2" file="$3" - local esc_key esc_val key_re - - esc_key=$(printf '%s' "$key" | sed -e 's/[][(){}.^$*+?|\\/]/\\&/g') - esc_val=$(printf '%s' "$encoded_val" | sed -e 's/[&|\\~]/\\&/g') - key_re="^[[:space:]]*${esc_key}([[:space:]]*[:=]|[[:space:]]+|[[:space:]]*$)" - if grep -qE "${key_re}" "${file}"; then - sed -ri "0,/${key_re}/!{/${key_re}/d;}" "${file}" - sed -ri "0,/${key_re}/s~${key_re}.*~${key}=${esc_val}~" "${file}" - else - printf '%s=%s\n' "$key" "$encoded_val" >> "${file}" - fi + PROPS_MODE=set PROPS_KEY="${key}" \ + PROPS_VALUE_ENCODED="${encoded_val}" PROPS_FILE="${file}" \ + awk -f "${PROPS_AWK}" /dev/null } set_prop() { @@ -70,12 +74,58 @@ set_prop() { get_prop_encoded() { local key="$1" file="$2" - local esc_key - esc_key=$(printf '%s' "$key" | sed -e 's/[][(){}.^$*+?|\\/]/\\&/g') - sed -nE \ - "s~^[[:space:]]*${esc_key}([[:space:]]*[:=][[:space:]]*|[[:space:]]+)(.*)$~\\2~p" \ - "${file}" | head -n 1 + PROPS_MODE=get PROPS_KEY="${key}" PROPS_FILE="${file}" \ + awk -f "${PROPS_AWK}" /dev/null +} + +# First uncommented `authenticator:` inside the gremlin-server.yaml +# authentication block. snakeyaml resolves duplicate top-level keys to the +# last one, but a mounted file carrying two authentication blocks is +# pathological; report the first and let the mismatch WARN handle it. +get_yaml_authenticator() { + local yaml="./conf/gremlin-server.yaml" + + [[ -f "${yaml}" ]] || return 0 + awk ' + /^[ \t]*#/ { next } + /^[ \t]*authentication[ \t]*:/ { inblk = 1; next } + inblk && /^[ \t]+authenticator[ \t]*:/ { + line = $0 + sub(/^[ \t]*authenticator[ \t]*:[ \t]*/, "", line) + sub(/[,:].*$/, "", line) + print line + exit + } + ' "./conf/gremlin-server.yaml" +} + +# enable-auth.sh appends definitions to files it did not write. On a +# mounted config those appended definitions are duplicates the two parsers +# resolve in opposite directions — HugeConfig (commons-configuration) takes +# the first, snakeyaml takes the last — so Gremlin and REST can land on +# different authenticators with no error from either. Normalize both sides +# to one definition of the same authenticator here; enable-auth.sh's +# per-file guards then make its appends no-ops on anything already set. +align_auth_config() { + local rest_auth yaml_auth + + rest_auth=$(get_prop_encoded "auth.authenticator" "${REST_SERVER_CONF}") + yaml_auth=$(get_yaml_authenticator) + if [[ -n "${rest_auth}" && -n "${yaml_auth}" && "${rest_auth}" != "${yaml_auth}" ]]; then + log "WARN: REST and Gremlin name different authenticators" \ + "('${rest_auth}' vs '${yaml_auth}'); leaving both untouched" + return + fi + if [[ -z "${rest_auth}" && -z "${yaml_auth}" ]]; then + export AUTHENTICATOR_CLASS="org.apache.hugegraph.auth.StandardAuthenticator" + elif [[ -n "${yaml_auth}" ]]; then + set_prop_encoded "auth.authenticator" "${yaml_auth}" "${REST_SERVER_CONF}" + else + export AUTHENTICATOR_CLASS="${rest_auth}" + fi + # auth.graph_store and the gremlin.graph flip are left to enable-auth.sh, + # which appends/rewrites only what is absent or still the plain default. } migrate_env() { @@ -147,6 +197,7 @@ elif [[ -n "${AUTH_TOKEN_SECRET_ENCODED}" ]]; then fi if [[ -n "${PASSWORD:-}" ]]; then set_prop "auth.admin_pa" "${PASSWORD}" "${REST_SERVER_CONF}" + align_auth_config # This script is idempotent and must run outside the initialization guard: # an upgrade can preserve the marker from an unauthenticated deployment. ./bin/enable-auth.sh diff --git a/hugegraph-server/hugegraph-dist/docker/props.awk b/hugegraph-server/hugegraph-dist/docker/props.awk new file mode 100644 index 0000000000..a7a3bde5e1 --- /dev/null +++ b/hugegraph-server/hugegraph-dist/docker/props.awk @@ -0,0 +1,227 @@ +# 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. +# +# props.awk — read and rewrite Java ".properties" files with the grammar +# HugeConfig (commons-configuration over JDK Properties) applies, so the +# entrypoint and the server agree on what a mounted file means. grep/sed +# rewrites do not: they see `\`-escaped keys, `:` separators, continuation +# lines and duplicate definitions differently, which is how a mounted +# config ends up with two definitions of one key. +# +# 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 +# variable, so secrets never appear in `ps` output or in awk's +# argv), and -v is not used for it so awk cannot mangle its +# backslash escapes. +# +# Grammar implemented (java.util.Properties line reader + the +# first-definition-wins rule Configuration.getString applies): +# - '#' / '!' comments and blank lines +# - '=' / ':' / whitespace separators, with whitespace then an optional +# single '=' or ':' accepted as one separator +# - continuations: a physical line ending in an odd number of +# backslashes joins the next line (its leading whitespace stripped) +# - backslash escapes in keys and values, including \uXXXX +# - duplicate logical keys resolve to the first definition +# +# Rewrites keep every untouched line byte-for-byte (comments, blank +# lines, unrelated entries), and replace the first definition where it +# stands, so mounted configs stay reviewable in git diffs. + +function die(msg) { + printf "props.awk: %s\n", msg > "/dev/stderr" + exit 1 +} + +function hex_digit(c) { + return index("0123456789abcdef", tolower(c)) - 1 +} + +# \uXXXX is a UTF-16 code unit in Java. Values here are effectively +# ISO-8859-1, so codes above 0xFF are kept as their literal escape text +# rather than being mangled through a single-byte sprintf. +function unescape(s, out, i, n, c, code, j, d, ok) { + out = "" + n = length(s) + for (i = 1; i <= n; i++) { + c = substr(s, i, 1) + if (c != "\\") { out = out c; continue } + if (i == n) break + i++ + c = substr(s, i, 1) + if (c == "u" && i + 4 <= n) { + code = 0 + ok = 1 + for (j = 1; j <= 4; j++) { + d = hex_digit(substr(s, i + j, 1)) + if (d < 0) { ok = 0; break } + code = code * 16 + d + } + if (ok) { + i += 4 + if (code <= 255) out = out sprintf("%c", code) + else out = out substr(s, i - 5, 6) + continue + } + } + if (c == "t") out = out "\t" + else if (c == "n") out = out "\n" + else if (c == "r") out = out "\r" + else if (c == "f") out = out "\f" + else out = out c + } + return out +} + +# A physical line is continued when it ends in an odd number of +# backslashes (an even count escapes itself). +function trailing_backslashes(s, n, k) { + n = length(s) + k = 0 + while (k < n && substr(s, n - k, 1) == "\\") k++ + return k +} + +function is_skipped(raw) { + return raw ~ /^[ \t]*([#!]|$)/ +} + +# Split a logical line into its raw (still-escaped) key and value parts. +# Results land in K_RAW / V_RAW because awk returns one value. +function split_kv(s, n, i, c, esc, sep_at, rest) { + n = length(s) + esc = 0 + sep_at = 0 + for (i = 1; i <= n; i++) { + c = substr(s, i, 1) + if (esc) { esc = 0; continue } + if (c == "\\") { esc = 1; continue } + if (c == "=" || c == ":" || c == " " || c == "\t") { sep_at = i; break } + } + if (sep_at == 0) { + K_RAW = s + V_RAW = "" + return + } + K_RAW = substr(s, 1, sep_at - 1) + rest = substr(s, sep_at) + c = substr(rest, 1, 1) + if (c == "=" || c == ":") { + rest = substr(rest, 2) + } else { + sub(/^[ \t]+/, "", rest) + c = substr(rest, 1, 1) + if (c == "=" || c == ":") rest = substr(rest, 2) + } + sub(/^[ \t]+/, "", rest) + V_RAW = rest +} + +# Load `file` into per-block arrays: one block per comment/blank line or +# logical entry, spanning exactly the physical lines it occupies. +function props_load(file, raw, nl, next_raw, start, logical) { + NLINES = 0 + while ((getline raw < file) > 0) { + NLINES++ + RAW[NLINES] = raw + } + close(file) + + NBLOCK = 0 + for (nl = 1; nl <= NLINES; nl++) { + raw = RAW[nl] + if (is_skipped(raw)) { + NBLOCK++ + BTYPE[NBLOCK] = "skip" + BFIRST[NBLOCK] = nl + BLAST[NBLOCK] = nl + continue + } + start = nl + logical = raw + while (trailing_backslashes(logical) % 2 == 1 && nl < NLINES) { + logical = substr(logical, 1, length(logical) - 1) + nl++ + next_raw = RAW[nl] + sub(/^[ \t]+/, "", next_raw) + logical = logical next_raw + } + split_kv(logical) + NBLOCK++ + BTYPE[NBLOCK] = "entry" + BFIRST[NBLOCK] = start + BLAST[NBLOCK] = nl + BKEY[NBLOCK] = unescape(K_RAW) + # Values stay in their on-disk escaped form. get Prop callers feed + # the result straight back into set, which would corrupt a decoded + # value by re-writing its backslashes as literals; keys are + # unescaped because they are matched against plain names. + BVAL[NBLOCK] = V_RAW + } +} + +function props_set(file, key, enc_val, b, first, ln) { + props_load(file) + first = 0 + for (b = 1; b <= NBLOCK; b++) { + if (BTYPE[b] == "entry" && BKEY[b] == key) { + if (first == 0) first = b + else BDROP[b] = 1 + } + } + for (b = 1; b <= NBLOCK; b++) { + if (BDROP[b]) continue + if (b == first) { + printf "%s=%s\n", key, enc_val > file + } else { + for (ln = BFIRST[b]; ln <= BLAST[b]; ln++) + print RAW[ln] > file + } + } + if (first == 0) + printf "%s=%s\n", key, enc_val > file + close(file) +} + +function props_get(file, key, b) { + props_load(file) + for (b = 1; b <= NBLOCK; b++) { + if (BTYPE[b] == "entry" && BKEY[b] == key) { + print BVAL[b] + return + } + } +} + +BEGIN { + mode = ENVIRON["PROPS_MODE"] + key = ENVIRON["PROPS_KEY"] + file = ENVIRON["PROPS_FILE"] + if (file == "" || key == "") + die("PROPS_FILE and PROPS_KEY must be set") + if (mode == "get") { + props_get(file, key) + } else if (mode == "set") { + props_set(file, key, ENVIRON["PROPS_VALUE_ENCODED"]) + } else { + die("PROPS_MODE must be get or set") + } +} diff --git a/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh index d5e11c5022..badf92a4f7 100644 --- a/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh @@ -23,10 +23,16 @@ entrypoint="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/docker-entrypoint.s test_dir="$(mktemp -d)" trap 'rm -rf "${test_dir}"' EXIT +# Eval the property helpers plus the PROPS_AWK location block they depend +# on. The entrypoint's top-level code hard-exits when props.awk is +# missing, so it cannot be sourced directly; anchor to the marker comment +# above the assignment instead. +PROPS_AWK="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/props.awk" +export PROPS_AWK eval "$(awk ' /^encode_prop_value\(\) \{/ { capture = 1 } capture { print } - capture && /^\}$/ && ++function_ends == 3 { exit } + capture && /^\}$/ && ++function_ends == 4 { exit } ' "${entrypoint}")" assert_replaced() { @@ -66,3 +72,52 @@ assert_line_count 1 \ "${duplicate_file}" assert_line_count 1 '^init_store\.enabled=true$' "${duplicate_file}" grep -q '^unrelated=true$' "${duplicate_file}" + +# An escaped key is one logical definition of that key, not a key with +# backslashes in its name: setting the plain key must rewrite it in place +# rather than appending a second definition whose only resolution is +# parser-dependent (and which HugeConfig then reports as a list). +escaped_file="${test_dir}/config-escaped-key" +printf '%s\n' \ + 'auth\.admin_pa=old' \ + 'unrelated=true' > "${escaped_file}" +set_prop "auth.admin_pa" "new" "${escaped_file}" +assert_line_count 1 '^auth\.admin_pa=new$' "${escaped_file}" +assert_line_count 1 '^unrelated=true$' "${escaped_file}" + +# A value continued onto the next line is part of the same definition: +# setting the key must remove the continuation, not leave it behind as a +# stray property of its own. +continued_file="${test_dir}/config-continuation" +printf '%s\n' \ + 'pd.peers 127.0.0.1:8686,\' \ + ' 127.0.0.2:8686' \ + 'unrelated=true' > "${continued_file}" +set_prop "pd.peers" "10.0.0.1:8686" "${continued_file}" +assert_line_count 1 '^pd\.peers=10\.0\.0\.1:8686$' "${continued_file}" +assert_line_count 1 '^unrelated=true$' "${continued_file}" +[[ "$(grep -c '127\.0\.0\.2' "${continued_file}")" -eq 0 ]] + +# get_prop_encoded reads through the same grammar: separators, escapes, +# continuations, and first-definition-wins duplicates. +get_file="${test_dir}/config-get" +printf '%s\n' \ + '#comment' \ + 'a\=b : colon value' \ + 'multiline first \' \ + ' second' \ + 'dup : one' \ + 'dup=two' > "${get_file}" +[[ "$(get_prop_encoded 'a=b' "${get_file}")" == "colon value" ]] +[[ "$(get_prop_encoded 'multiline' "${get_file}")" == "first second" ]] +[[ "$(get_prop_encoded 'dup' "${get_file}")" == "one" ]] + +# Appends must still happen when the file has no definition of the key, +# including when the only occurrences are inside comments. +append_file="${test_dir}/config-append" +printf '%s\n' \ + '#init_store.enabled=false' \ + 'unrelated=true' > "${append_file}" +set_prop "init_store.enabled" "true" "${append_file}" +assert_line_count 1 '^init_store\.enabled=true$' "${append_file}" +assert_line_count 1 '^#init_store\.enabled=false$' "${append_file}" diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh index fcdadd906f..119be9f979 100644 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh @@ -41,16 +41,34 @@ if [ ! -d "$BAK_CONF" ]; then cp "${CONF}/${GREMLIN_SERVER_CONF}" "${BAK_CONF}/${GREMLIN_SERVER_CONF}.bak" cp "${CONF}/${REST_SERVER_CONF}" "${BAK_CONF}/${REST_SERVER_CONF}.bak" cp "${CONF}/graphs/${GRAPH_CONF}" "${BAK_CONF}/${GRAPH_CONF}.bak" +fi + +# The appends below are guarded per file and match only an absent or still +# commented-out definition, so they are no-ops on any config that already +# carries authentication (e.g. a mounted one, or a re-run of this script). +# Appending unconditionally used to create duplicate definitions that the +# properties parser (first definition wins) and the yaml parser (last wins) +# resolved in opposite directions, leaving Gremlin and REST on different +# authenticators. +AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}" +if ! grep -Eq '^[ \t]*authentication[ \t]*:' "${CONF}/${GREMLIN_SERVER_CONF}"; then sed -i -e '$a\authentication: {' \ - -e '$a\ authenticator: org.apache.hugegraph.auth.StandardAuthenticator,' \ + -e "\$a\\ authenticator: ${AUTHENTICATOR_CLASS}," \ -e '$a\ authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler,' \ -e '$a\ config: {tokens: conf/rest-server.properties}' \ -e '$a\}' ${CONF}/${GREMLIN_SERVER_CONF} +fi - sed -i -e '$a\auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' \ - -e '$a\auth.graph_store=hugegraph' ${CONF}/${REST_SERVER_CONF} +if ! grep -Eq '^[ \t]*auth\.authenticator[ \t]*=' "${CONF}/${REST_SERVER_CONF}"; then + sed -i -e "\$a\\auth.authenticator=${AUTHENTICATOR_CLASS}" ${CONF}/${REST_SERVER_CONF} +fi + +if ! grep -Eq '^[ \t]*auth\.graph_store[ \t]*=' "${CONF}/${REST_SERVER_CONF}"; then + sed -i -e '$a\auth.graph_store=hugegraph' ${CONF}/${REST_SERVER_CONF} +fi - sed -i 's/gremlin.graph=org.apache.hugegraph.HugeFactory/gremlin.graph=org.apache.hugegraph.auth.HugeFactoryAuthProxy/g' ${CONF}/graphs/${GRAPH_CONF} +if grep -Eq '^gremlin\.graph[ \t]*=org\.apache\.hugegraph\.HugeFactory[ \t]*$' "${CONF}/graphs/${GRAPH_CONF}"; then + sed -i 's/^gremlin\.graph[ \t]*=org\.apache\.hugegraph\.HugeFactory[ \t]*$/gremlin.graph=org.apache.hugegraph.auth.HugeFactoryAuthProxy/' ${CONF}/graphs/${GRAPH_CONF} fi From f5e368cea2cabab7692bf36597096a7414844427 Mon Sep 17 00:00:00 2001 From: Adarsh Date: Fri, 11 Sep 2026 22:31:36 +0530 Subject: [PATCH 2/2] fix(docker): close review gaps in auth bootstrap alignment Review follow-ups on the props.awk bootstrap: the yaml authenticator scalar now goes through a snakeyaml-shaped cleanup (inline comments, quotes and padding stripped) instead of only cutting at the first comma or colon; a flow mapping on the authentication line itself is read, and an authentication block without a readable authenticator takes the WARN branch instead of the both-empty default. props.awk strips leading whitespace before the key the way java.util.Properties does, so an indented key is rewritten in place rather than duplicated. enable-auth.sh's append guards now accept the ':', bare-whitespace and backslash-escaped spellings with [[:blank:]] classes (the '[ \t]' bracket matched space, backslash and the letter t), and the gremlin.graph flip embeds the carriage return as a byte because GNU grep reads \r in a pattern as the letter r, which made the anchored guard drop mounted CRLF configs. Test docs name the environment variables and the function count they rely on, and new regression tests cover indented keys, yaml scalar cleanup, flow mappings and the block-without-authenticator WARN. --- .../docker/docker-entrypoint.sh | 64 +++++++++++++-- .../hugegraph-dist/docker/props.awk | 13 +++- .../docker/test/test-docker-entrypoint.sh | 77 ++++++++++++++++--- .../src/assembly/static/bin/enable-auth.sh | 21 +++-- 4 files changed, 149 insertions(+), 26 deletions(-) diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh index ee2994776c..ce44f35915 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh @@ -80,24 +80,69 @@ get_prop_encoded() { } # First uncommented `authenticator:` inside the gremlin-server.yaml -# authentication block. snakeyaml resolves duplicate top-level keys to the -# last one, but a mounted file carrying two authentication blocks is -# pathological; report the first and let the mismatch WARN handle it. +# authentication block, or on the `authentication:` line itself (a flow +# mapping). snakeyaml resolves duplicate top-level keys to the last one, +# but a mounted file carrying two authentication blocks is pathological; +# report the first and let the mismatch WARN handle it. The scalar is +# cleaned the way snakeyaml reads it — an inline comment (a '#' preceded +# by whitespace), surrounding quotes and padding are stripped — because +# java.util.Properties keeps all of those in the class name. get_yaml_authenticator() { local yaml="./conf/gremlin-server.yaml" [[ -f "${yaml}" ]] || return 0 awk ' + function scalar(s, out, i, n, c, q) { + out = "" + q = "" + n = length(s) + for (i = 1; i <= n; i++) { + c = substr(s, i, 1) + if (q != "") { + if (c == q) q = "" + else out = out c + continue + } + if (c == "\"" || c == "\047") { q = c; continue } + if (c == "#" && + (out == "" || substr(out, length(out), 1) ~ /[ \t]/)) + break + if (c == "," || c == "}" || c == "]") break + out = out c + } + sub(/^[ \t\r]+/, "", out) + sub(/[ \t\r]+$/, "", out) + return out + } /^[ \t]*#/ { next } - /^[ \t]*authentication[ \t]*:/ { inblk = 1; next } + /^[ \t]*authentication[ \t]*:/ { + inblk = 1 + line = $0 + sub(/^[ \t]*authentication[ \t]*:[ \t]*/, "", line) + if (match(line, /authenticator[ \t]*:/)) { + print scalar(substr(line, RSTART + RLENGTH)) + exit + } + next + } inblk && /^[ \t]+authenticator[ \t]*:/ { line = $0 sub(/^[ \t]*authenticator[ \t]*:[ \t]*/, "", line) - sub(/[,:].*$/, "", line) - print line + print scalar(line) exit } - ' "./conf/gremlin-server.yaml" + ' "${yaml}" +} + +# A mounted yaml can carry an authentication block whose authenticator +# cannot be read (an empty or unparseable one). That is not the +# both-empty case: exporting the default would override an explicit +# choice that snakeyaml does resolve, so callers treat it as a mismatch. +has_yaml_authentication_block() { + local yaml="./conf/gremlin-server.yaml" + + [[ -f "${yaml}" ]] || return 1 + grep -Eq '^[[:blank:]]*authentication[[:blank:]]*:' "${yaml}" } # enable-auth.sh appends definitions to files it did not write. On a @@ -112,6 +157,11 @@ align_auth_config() { rest_auth=$(get_prop_encoded "auth.authenticator" "${REST_SERVER_CONF}") yaml_auth=$(get_yaml_authenticator) + if [[ -z "${yaml_auth}" ]] && has_yaml_authentication_block; then + log "WARN: gremlin-server.yaml carries an authentication block" \ + "without a readable authenticator; leaving both sides untouched" + return + fi if [[ -n "${rest_auth}" && -n "${yaml_auth}" && "${rest_auth}" != "${yaml_auth}" ]]; then log "WARN: REST and Gremlin name different authenticators" \ "('${rest_auth}' vs '${yaml_auth}'); leaving both untouched" diff --git a/hugegraph-server/hugegraph-dist/docker/props.awk b/hugegraph-server/hugegraph-dist/docker/props.awk index a7a3bde5e1..738efced25 100644 --- a/hugegraph-server/hugegraph-dist/docker/props.awk +++ b/hugegraph-server/hugegraph-dist/docker/props.awk @@ -20,14 +20,14 @@ # lines and duplicate definitions differently, which is how a mounted # config ends up with two definitions of one key. # -# One invocation, selected with the `mode` environment variable: +# One invocation, selected with the `PROPS_MODE` environment variable: # -# mode=get key=K file=F +# PROPS_MODE=get PROPS_KEY=K PROPS_FILE=F # print the value of K's first logical definition -# mode=set key=K file=F +# 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 PROP_VALUE_ENCODED (an environment +# value arrives pre-encoded in PROPS_VALUE_ENCODED (an environment # variable, so secrets never appear in `ps` output or in awk's # argv), and -v is not used for it so awk cannot mangle its # backslash escapes. @@ -164,6 +164,11 @@ function props_load(file, raw, nl, next_raw, start, logical) { sub(/^[ \t]+/, "", next_raw) logical = logical next_raw } + # java.util.Properties ignores whitespace before the key; strip it + # so split_kv's separator scan agrees (an indented key used to be + # read as a key whose name started with a space, and a set then + # appended a second definition of the real key). + sub(/^[ \t]+/, "", logical) split_kv(logical) NBLOCK++ BTYPE[NBLOCK] = "entry" diff --git a/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh index badf92a4f7..386e70ed62 100644 --- a/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh @@ -23,17 +23,21 @@ entrypoint="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/docker-entrypoint.s test_dir="$(mktemp -d)" trap 'rm -rf "${test_dir}"' EXIT -# Eval the property helpers plus the PROPS_AWK location block they depend -# on. The entrypoint's top-level code hard-exits when props.awk is -# missing, so it cannot be sourced directly; anchor to the marker comment -# above the assignment instead. +# Eval the property and yaml helpers one by one. The entrypoint's +# top-level code hard-exits when props.awk is missing, so it cannot be +# sourced directly; extracting by function name keeps this independent of +# helper order. PROPS_AWK is recomputed below. +for fn in encode_prop_value set_prop_encoded set_prop get_prop_encoded \ + get_yaml_authenticator has_yaml_authentication_block align_auth_config; do + eval "$(awk -v fn="${fn}" ' + index($0, fn "() {") == 1 { capture = 1 } + capture { print } + capture && /^}$/ { exit } + ' "${entrypoint}")" +done +log() { echo "[hugegraph-server-entrypoint] $*"; } PROPS_AWK="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/props.awk" export PROPS_AWK -eval "$(awk ' - /^encode_prop_value\(\) \{/ { capture = 1 } - capture { print } - capture && /^\}$/ && ++function_ends == 4 { exit } -' "${entrypoint}")" assert_replaced() { local separator="$1" @@ -121,3 +125,58 @@ printf '%s\n' \ set_prop "init_store.enabled" "true" "${append_file}" assert_line_count 1 '^init_store\.enabled=true$' "${append_file}" assert_line_count 1 '^#init_store\.enabled=false$' "${append_file}" + +# A key indented with leading whitespace is still one definition of the +# key: java.util.Properties ignores whitespace before a key, so an +# indented key must be read and rewritten in place rather than duplicated. +indented_file="${test_dir}/config-indented-key" +printf '%s\n' \ + ' auth.token_secret: old-secret' \ + 'unrelated=true' > "${indented_file}" +[[ "$(get_prop_encoded 'auth.token_secret' "${indented_file}")" == "old-secret" ]] +set_prop_encoded 'auth.token_secret' 'new-secret' "${indented_file}" +assert_line_count 1 'auth\.token_secret' "${indented_file}" +assert_line_count 1 '^unrelated=true$' "${indented_file}" + +# get_yaml_authenticator must agree with snakeyaml on what a mounted +# gremlin-server.yaml says: the authenticator inside the authentication +# block — quoted scalars and inline comments cleaned the way snakeyaml +# strips them — and a flow mapping on the authentication line itself. +# align_auth_config must not read an authentication block without a +# readable authenticator as "no yaml side": exporting the default there +# would override an explicit choice, so both sides stay untouched. +yaml_dir="${test_dir}/yaml" +mkdir -p "${yaml_dir}/conf" +( + cd "${yaml_dir}" || exit 1 + REST_SERVER_CONF="./conf/rest-server.properties" + : > "${REST_SERVER_CONF}" + + printf '%s\n' \ + 'authentication:' \ + ' authenticator: "com.example.MyAuth" # custom' \ + ' authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler' \ + > conf/gremlin-server.yaml + [[ "$(get_yaml_authenticator)" == "com.example.MyAuth" ]] + + printf '%s\n' \ + 'authentication: {authenticator: com.example.FlowAuth, authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler, config: {tokens: conf/rest-server.properties}}' \ + > conf/gremlin-server.yaml + [[ "$(get_yaml_authenticator)" == "com.example.FlowAuth" ]] + + printf '%s\n' \ + 'authentication:' \ + ' authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler' \ + > conf/gremlin-server.yaml + unset AUTHENTICATOR_CLASS + align_auth_config + [[ -z "${AUTHENTICATOR_CLASS:-}" ]] + [[ ! -s "${REST_SERVER_CONF}" ]] + + printf '%s\n' \ + 'authentication:' \ + ' authenticator: com.example.YamlAuth' \ + > conf/gremlin-server.yaml + align_auth_config + grep -q '^auth\.authenticator=com\.example\.YamlAuth$' "${REST_SERVER_CONF}" +) diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh index 119be9f979..e6a3c01513 100644 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh @@ -46,14 +46,18 @@ fi # The appends below are guarded per file and match only an absent or still # commented-out definition, so they are no-ops on any config that already # carries authentication (e.g. a mounted one, or a re-run of this script). -# Appending unconditionally used to create duplicate definitions that the +# The guards accept every spelling java.util.Properties reads as the key — +# '=' or ':' or bare-whitespace separators, leading whitespace and +# backslash-escaped dots — and the gremlin.graph flip tolerates CRLF +# endings, which a mounted config saved on Windows carries. Appending +# unconditionally used to create duplicate definitions that the # properties parser (first definition wins) and the yaml parser (last wins) # resolved in opposite directions, leaving Gremlin and REST on different # authenticators. AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}" -if ! grep -Eq '^[ \t]*authentication[ \t]*:' "${CONF}/${GREMLIN_SERVER_CONF}"; then +if ! grep -Eq '^[[:blank:]]*authentication[[:blank:]]*:' "${CONF}/${GREMLIN_SERVER_CONF}"; then sed -i -e '$a\authentication: {' \ -e "\$a\\ authenticator: ${AUTHENTICATOR_CLASS}," \ -e '$a\ authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler,' \ @@ -61,14 +65,19 @@ if ! grep -Eq '^[ \t]*authentication[ \t]*:' "${CONF}/${GREMLIN_SERVER_CONF}"; t -e '$a\}' ${CONF}/${GREMLIN_SERVER_CONF} fi -if ! grep -Eq '^[ \t]*auth\.authenticator[ \t]*=' "${CONF}/${REST_SERVER_CONF}"; then +if ! grep -Eq '^[[:blank:]]*auth[\\]?\.authenticator[[:blank:]]*([:=]|[[:blank:]])' "${CONF}/${REST_SERVER_CONF}"; then sed -i -e "\$a\\auth.authenticator=${AUTHENTICATOR_CLASS}" ${CONF}/${REST_SERVER_CONF} fi -if ! grep -Eq '^[ \t]*auth\.graph_store[ \t]*=' "${CONF}/${REST_SERVER_CONF}"; then +if ! grep -Eq '^[[:blank:]]*auth[\\]?\.graph_store[[:blank:]]*([:=]|[[:blank:]])' "${CONF}/${REST_SERVER_CONF}"; then sed -i -e '$a\auth.graph_store=hugegraph' ${CONF}/${REST_SERVER_CONF} fi -if grep -Eq '^gremlin\.graph[ \t]*=org\.apache\.hugegraph\.HugeFactory[ \t]*$' "${CONF}/graphs/${GRAPH_CONF}"; then - sed -i 's/^gremlin\.graph[ \t]*=org\.apache\.hugegraph\.HugeFactory[ \t]*$/gremlin.graph=org.apache.hugegraph.auth.HugeFactoryAuthProxy/' ${CONF}/graphs/${GRAPH_CONF} +# GNU grep reads \r in a pattern as the letter r, so the carriage return a +# CRLF line ends with is embedded as a byte: without it the anchored guard +# misses a mounted CRLF config and the factory is never wrapped for auth +# although both servers already believe authentication is on. +CR=$'\r' +if grep -Eq "^gremlin\\.graph[[:blank:]]*=org\\.apache\\.hugegraph\\.HugeFactory[[:blank:]]*${CR}?\$" "${CONF}/graphs/${GRAPH_CONF}"; then + sed -i 's/^\(gremlin\.graph[[:blank:]]*=[[:blank:]]*\)org\.apache\.hugegraph\.HugeFactory/\1org.apache.hugegraph.auth.HugeFactoryAuthProxy/' "${CONF}/graphs/${GRAPH_CONF}" fi