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 01/11] 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 02/11] 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 From 5f5051130d29089e34fec1d611d52efa5ba566b6 Mon Sep 17 00:00:00 2001 From: Adarsh-Me Date: Sat, 12 Sep 2026 09:07:14 +0000 Subject: [PATCH 03/11] fix(docker): close review gaps in auth bootstrap parsing and rewrite Address review 5185689081 on the auth bootstrap alignment: - props.awk: strip one trailing CR while assembling logical lines so CRLF configs parse like java.util.Properties, without touching the RAW bytes replayed on rewrite; add get-decoded mode. - props.awk: die when getline fails and rewrite atomically through a sibling temp file renamed over the original. - enable-auth.sh: widen the gremlin.graph guard and flip together for colon, equals, bare-whitespace, leading-blank and escaped-dot spellings with optional CR, still skipping proxied/commented lines. - docker-entrypoint.sh: compare the unescaped authenticator with the yaml scalar and write the yaml side through the encoding setter. Add CRLF plus escaped-authenticator regression cases to test-docker-entrypoint.sh. --- .../docker/docker-entrypoint.sh | 15 +++++- .../hugegraph-dist/docker/props.awk | 51 +++++++++++++++---- .../docker/test/test-docker-entrypoint.sh | 46 ++++++++++++++++- .../src/assembly/static/bin/enable-auth.sh | 4 +- 4 files changed, 101 insertions(+), 15 deletions(-) diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh index ce44f35915..bff61f6977 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh @@ -79,6 +79,17 @@ get_prop_encoded() { awk -f "${PROPS_AWK}" /dev/null } +# Decoded read: unescapes the on-disk value the way java.util.Properties +# does, so it compares equal with the snakeyaml-decoded scalar from +# get_yaml_authenticator. The raw get_prop_encoded mode stays for the +# secret round trip, which must replay backslashes byte-for-byte. +get_prop() { + local key="$1" file="$2" + + PROPS_MODE=get-decoded PROPS_KEY="${key}" PROPS_FILE="${file}" \ + awk -f "${PROPS_AWK}" /dev/null +} + # First uncommented `authenticator:` inside the gremlin-server.yaml # authentication block, or on the `authentication:` line itself (a flow # mapping). snakeyaml resolves duplicate top-level keys to the last one, @@ -155,7 +166,7 @@ has_yaml_authentication_block() { align_auth_config() { local rest_auth yaml_auth - rest_auth=$(get_prop_encoded "auth.authenticator" "${REST_SERVER_CONF}") + rest_auth=$(get_prop "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" \ @@ -170,7 +181,7 @@ align_auth_config() { 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}" + set_prop "auth.authenticator" "${yaml_auth}" "${REST_SERVER_CONF}" else export AUTHENTICATOR_CLASS="${rest_auth}" fi diff --git a/hugegraph-server/hugegraph-dist/docker/props.awk b/hugegraph-server/hugegraph-dist/docker/props.awk index 738efced25..10cb0dc7eb 100644 --- a/hugegraph-server/hugegraph-dist/docker/props.awk +++ b/hugegraph-server/hugegraph-dist/docker/props.awk @@ -135,20 +135,32 @@ function split_kv(s, n, i, c, esc, sep_at, rest) { V_RAW = rest } +function shquote(s) { + gsub(/'/, "'\\''", s) + return "'" s "'" +} + # 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) { +function props_load(file, raw, rc, nl, stripped, next_raw, start, logical) { NLINES = 0 - while ((getline raw < file) > 0) { + while ((rc = (getline raw < file)) > 0) { NLINES++ RAW[NLINES] = raw } + if (rc == -1) + die("cannot read " file) close(file) NBLOCK = 0 for (nl = 1; nl <= NLINES; nl++) { raw = RAW[nl] - if (is_skipped(raw)) { + # CRLF: java.util.Properties drops the line terminator, so one + # trailing CR is stripped for parsing only. RAW[] keeps the byte + # so props_set replays untouched lines byte-for-byte. + stripped = raw + sub(/\r$/, "", stripped) + if (is_skipped(stripped)) { NBLOCK++ BTYPE[NBLOCK] = "skip" BFIRST[NBLOCK] = nl @@ -156,11 +168,12 @@ function props_load(file, raw, nl, next_raw, start, logical) { continue } start = nl - logical = raw + logical = stripped while (trailing_backslashes(logical) % 2 == 1 && nl < NLINES) { logical = substr(logical, 1, length(logical) - 1) nl++ next_raw = RAW[nl] + sub(/\r$/, "", next_raw) sub(/^[ \t]+/, "", next_raw) logical = logical next_raw } @@ -183,7 +196,7 @@ function props_load(file, raw, nl, next_raw, start, logical) { } } -function props_set(file, key, enc_val, b, first, ln) { +function props_set(file, key, enc_val, tmp, cmd, b, first, ln) { props_load(file) first = 0 for (b = 1; b <= NBLOCK; b++) { @@ -192,18 +205,24 @@ function props_set(file, key, enc_val, b, first, ln) { else BDROP[b] = 1 } } + # Atomic rewrite: the original is never truncated. Everything lands + # in a sibling temp file that is closed and renamed over the original. + tmp = file ".tmp" for (b = 1; b <= NBLOCK; b++) { if (BDROP[b]) continue if (b == first) { - printf "%s=%s\n", key, enc_val > file + printf "%s=%s\n", key, enc_val > tmp } else { for (ln = BFIRST[b]; ln <= BLAST[b]; ln++) - print RAW[ln] > file + print RAW[ln] > tmp } } if (first == 0) - printf "%s=%s\n", key, enc_val > file - close(file) + printf "%s=%s\n", key, enc_val > tmp + close(tmp) + cmd = "mv -- " shquote(tmp) " " shquote(file) + if (system(cmd) != 0) + die("cannot rename " tmp " over " file) } function props_get(file, key, b) { @@ -216,6 +235,16 @@ function props_get(file, key, b) { } } +function props_get_decoded(file, key, b) { + props_load(file) + for (b = 1; b <= NBLOCK; b++) { + if (BTYPE[b] == "entry" && BKEY[b] == key) { + print unescape(BVAL[b]) + return + } + } +} + BEGIN { mode = ENVIRON["PROPS_MODE"] key = ENVIRON["PROPS_KEY"] @@ -224,9 +253,11 @@ BEGIN { die("PROPS_FILE and PROPS_KEY must be set") if (mode == "get") { props_get(file, key) + } else if (mode == "get-decoded") { + props_get_decoded(file, key) } else if (mode == "set") { props_set(file, key, ENVIRON["PROPS_VALUE_ENCODED"]) } else { - die("PROPS_MODE must be get or set") + die("PROPS_MODE must be get, get-decoded 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 386e70ed62..51eabe5746 100644 --- a/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh @@ -27,7 +27,7 @@ trap 'rm -rf "${test_dir}"' EXIT # 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 \ +for fn in encode_prop_value set_prop_encoded set_prop get_prop_encoded get_prop \ get_yaml_authenticator has_yaml_authentication_block align_auth_config; do eval "$(awk -v fn="${fn}" ' index($0, fn "() {") == 1 { capture = 1 } @@ -180,3 +180,47 @@ mkdir -p "${yaml_dir}/conf" align_auth_config grep -q '^auth\.authenticator=com\.example\.YamlAuth$' "${REST_SERVER_CONF}" ) + +# CRLF (Windows-saved) configs parse the way java.util.Properties reads +# them: one trailing CR is a line terminator, not part of the value, and +# a backslash before CRLF still continues the value onto the next line. +# Untouched lines keep their CR bytes on rewrite. +crlf_file="${test_dir}/config-crlf" +printf 'auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator\r\n' > "${crlf_file}" +printf 'pd.peers=a,\\\r\n b\r\n' >> "${crlf_file}" +printf 'unrelated=true\r\n' >> "${crlf_file}" +[[ "$(get_prop_encoded 'auth.authenticator' "${crlf_file}")" == \ + "org.apache.hugegraph.auth.StandardAuthenticator" ]] +[[ "$(get_prop_encoded 'pd.peers' "${crlf_file}")" == "a,b" ]] +[[ "$(get_prop 'auth.authenticator' "${crlf_file}")" == \ + "org.apache.hugegraph.auth.StandardAuthenticator" ]] +set_prop 'auth.authenticator' 'com.example.NewAuth' "${crlf_file}" +grep -q '^auth\.authenticator=com\.example\.NewAuth$' "${crlf_file}" +[[ "$(get_prop_encoded 'pd.peers' "${crlf_file}")" == "a,b" ]] +if ! grep -q $'^unrelated=true\r$' "${crlf_file}"; then + echo "CRLF bytes of untouched lines must be preserved" >&2 + exit 1 +fi + +# An escaped authenticator and a plain yaml scalar name the same class: +# the comparison unescapes first, so no spurious WARN and no skipped +# alignment. +escaped_auth_dir="${test_dir}/yaml-escaped-auth" +mkdir -p "${escaped_auth_dir}/conf" +( + cd "${escaped_auth_dir}" || exit 1 + REST_SERVER_CONF="./conf/rest-server.properties" + printf '%s\n' \ + 'auth.authenticator=org.apache.hugegraph.auth\.StandardAuthenticator' \ + > "${REST_SERVER_CONF}" + printf '%s\n' \ + 'authentication:' \ + ' authenticator: org.apache.hugegraph.auth.StandardAuthenticator' \ + > conf/gremlin-server.yaml + unset AUTHENTICATOR_CLASS + align_out=$(align_auth_config 2>&1) + [[ -z "${AUTHENTICATOR_CLASS:-}" ]] + [[ "${align_out}" != *"different authenticators"* ]] + grep -q '^auth\.authenticator=org\.apache\.hugegraph\.auth\.StandardAuthenticator$' \ + "${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 e6a3c01513..8524894f26 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 @@ -78,6 +78,6 @@ fi # 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}" +if grep -Eq "^[[:blank:]]*gremlin[\\\\]?\\.graph[[:blank:]]*([:=]|[[:blank:]])[[:blank:]]*org\\.apache\\.hugegraph\\.HugeFactory[[:blank:]]*${CR}?$" "${CONF}/graphs/${GRAPH_CONF}"; then + sed -i -E "s#^([[:blank:]]*gremlin[\\\\]?\\.graph[[:blank:]]*([:=]|[[:blank:]])[[:blank:]]*)org\\.apache\\.hugegraph\\.HugeFactory#\\1org.apache.hugegraph.auth.HugeFactoryAuthProxy#" "${CONF}/graphs/${GRAPH_CONF}" fi From bedc21eb85631185d5d09fff2e8e03a898ae9de0 Mon Sep 17 00:00:00 2001 From: Adarsh Date: Sun, 13 Sep 2026 11:50:43 +0530 Subject: [PATCH 04/11] fix(docker): keep the config inode when rewriting properties The staged temp file was renamed over the config, replacing its inode: a 0600 config holding secrets came back umask-world-readable, a symlinked config was replaced by a regular file, and a config bind-mounted as a single file could not be renamed over at all (rename(2) returns EBUSY on a mount point), aborting the entrypoint on exactly the mounted configs this path exists for. The temp file is now copied back onto the original instead, which keeps the inode, mode, symlink and mount point, and is created 0600 itself since it can hold secrets while it exists. Regression tests check that a 0600 file keeps its mode and that a symlink survives a set with its target rewritten. --- .../hugegraph-dist/docker/props.awk | 19 +++++++++++++---- .../docker/test/test-docker-entrypoint.sh | 21 +++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/hugegraph-server/hugegraph-dist/docker/props.awk b/hugegraph-server/hugegraph-dist/docker/props.awk index 10cb0dc7eb..29b214d9b5 100644 --- a/hugegraph-server/hugegraph-dist/docker/props.awk +++ b/hugegraph-server/hugegraph-dist/docker/props.awk @@ -205,8 +205,9 @@ function props_set(file, key, enc_val, tmp, cmd, b, first, ln) { else BDROP[b] = 1 } } - # Atomic rewrite: the original is never truncated. Everything lands - # in a sibling temp file that is closed and renamed over the original. + # Staged rewrite: everything lands in a sibling temp file first, so a + # failure before the copy-back leaves the original untouched. The temp + # file can hold secrets, so it is created 0600 regardless of the umask. tmp = file ".tmp" for (b = 1; b <= NBLOCK; b++) { if (BDROP[b]) continue @@ -220,9 +221,19 @@ function props_set(file, key, enc_val, tmp, cmd, b, first, ln) { if (first == 0) printf "%s=%s\n", key, enc_val > tmp close(tmp) - cmd = "mv -- " shquote(tmp) " " shquote(file) + # Copy the completed temp file back onto the original instead of + # renaming it: a rename replaces the inode, which would lose the + # file's permissions (a 0600 config holding secrets would come back + # umask-world-readable), turn a symlinked config into a regular file, + # and fail with EBUSY on a config bind-mounted as a single file — the + # mounted case this path exists for. The copy keeps the inode, mode, + # symlink and mount point, and since the temp file is fully written + # before the original is truncated, a failed copy still leaves the + # previous content on disk. + system("chmod 600 -- " shquote(tmp)) + cmd = "cat -- " shquote(tmp) " > " shquote(file) " && rm -f -- " shquote(tmp) if (system(cmd) != 0) - die("cannot rename " tmp " over " file) + die("cannot copy " tmp " back over " file) } function props_get(file, key, b) { 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 51eabe5746..7304e05f44 100644 --- a/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh @@ -224,3 +224,24 @@ mkdir -p "${escaped_auth_dir}/conf" grep -q '^auth\.authenticator=org\.apache\.hugegraph\.auth\.StandardAuthenticator$' \ "${REST_SERVER_CONF}" ) + +# A set must keep the config's inode: a copy-back preserves the file's +# permissions (a 0600 config holding secrets must not come back +# umask-readable) and leaves a symlinked config pointing at its target +# instead of replacing it with a regular file. +mode_file="${test_dir}/config-mode" +printf '%s\n' 'unrelated=true' > "${mode_file}" +chmod 600 "${mode_file}" +set_prop "init_store.enabled" "true" "${mode_file}" +[[ "$(stat -c '%a' "${mode_file}")" == "600" ]] +grep -q '^init_store\.enabled=true$' "${mode_file}" +grep -q '^unrelated=true$' "${mode_file}" +[[ ! -e "${mode_file}.tmp" ]] + +target_file="${test_dir}/config-target" +link_file="${test_dir}/config-link" +printf '%s\n' 'unrelated=true' > "${target_file}" +ln -s "${target_file}" "${link_file}" +set_prop "init_store.enabled" "true" "${link_file}" +[[ -L "${link_file}" ]] +grep -q '^init_store\.enabled=true$' "${target_file}" From bf2718f54b9df8bb5175c86b29782ca17d5f6023 Mon Sep 17 00:00:00 2001 From: Oracle Public Cloud User Date: Wed, 16 Sep 2026 13:26:23 +0000 Subject: [PATCH 05/11] fix(docker): refuse one-sided auth bootstrap and pre-create tmp 0600 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unreadable-authenticator yaml block: align_auth_config now fails the entrypoint instead of logging 'leaving both sides untouched' while enable-auth.sh goes on to write the REST side alone (REST on StandardAuthenticator vs Gremlin on AllowAllAuthenticator). The error tells the operator to add an 'authenticator:' entry or remove the block. props.awk: pre-create the rewrite temp file 0600 (umask 077) before the first write so secrets never sit briefly umask-readable; the chmod after close is kept for stale tmp files from crashed runs. Tests: the unreadable-block case now asserts refusal, and a new case runs enable-auth.sh against the same layout to prove it would write only REST (yaml untouched, graph flipped) — i.e. what the refusal prevents. --- .../docker/docker-entrypoint.sh | 16 ++++-- .../hugegraph-dist/docker/props.awk | 8 ++- .../docker/test/test-docker-entrypoint.sh | 54 +++++++++++++++++-- 3 files changed, 70 insertions(+), 8 deletions(-) diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh index bff61f6977..63f2c9cd07 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh @@ -169,9 +169,17 @@ align_auth_config() { rest_auth=$(get_prop "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 + # Refuse instead of bootstrapping one side: enable-auth.sh runs right + # after align and only touches the REST side, so continuing would put + # REST on StandardAuthenticator while Gremlin stays on TinkerPop's + # AllowAllAuthenticator default. Failing fast (rather than skipping + # enable-auth.sh) keeps a PASSWORD deployment from starting with + # authentication silently half-applied. + log "ERROR: gremlin-server.yaml carries an authentication block" \ + "without a readable authenticator; refusing to bootstrap" \ + "authentication one-sided. Add an 'authenticator:' entry to" \ + "the block or remove the block, then restart." + return 1 fi if [[ -n "${rest_auth}" && -n "${yaml_auth}" && "${rest_auth}" != "${yaml_auth}" ]]; then log "WARN: REST and Gremlin name different authenticators" \ @@ -258,6 +266,8 @@ elif [[ -n "${AUTH_TOKEN_SECRET_ENCODED}" ]]; then fi if [[ -n "${PASSWORD:-}" ]]; then set_prop "auth.admin_pa" "${PASSWORD}" "${REST_SERVER_CONF}" + # A refusal inside align_auth_config exits the entrypoint under set -e, + # so enable-auth.sh can never run one-sided after it. align_auth_config # This script is idempotent and must run outside the initialization guard: # an upgrade can preserve the marker from an unauthenticated deployment. diff --git a/hugegraph-server/hugegraph-dist/docker/props.awk b/hugegraph-server/hugegraph-dist/docker/props.awk index 29b214d9b5..621cd480a8 100644 --- a/hugegraph-server/hugegraph-dist/docker/props.awk +++ b/hugegraph-server/hugegraph-dist/docker/props.awk @@ -207,8 +207,14 @@ function props_set(file, key, enc_val, tmp, cmd, b, first, ln) { } # Staged rewrite: everything lands in a sibling temp file first, so a # failure before the copy-back leaves the original untouched. The temp - # file can hold secrets, so it is created 0600 regardless of the umask. + # file can hold secrets, so it is pre-created 0600 before the first + # write: awk's `>` below would otherwise create it under the process + # umask (usually 0644), leaving auth.admin_pa or auth.token_secret + # briefly group- and world-readable. Truncating an existing file keeps + # its mode, and the chmod after close repairs a stale tmp left behind + # by a crashed run. tmp = file ".tmp" + system("umask 077 && : > " shquote(tmp)) for (b = 1; b <= NBLOCK; b++) { if (BDROP[b]) continue if (b == first) { 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 7304e05f44..012cf8907a 100644 --- a/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh @@ -142,9 +142,10 @@ assert_line_count 1 '^unrelated=true$' "${indented_file}" # 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. +# align_auth_config refuses an authentication block without a readable +# authenticator instead of treating it as "no yaml side": exporting the +# default there would override an explicit choice, and continuing would let +# enable-auth.sh write the REST side alone. yaml_dir="${test_dir}/yaml" mkdir -p "${yaml_dir}/conf" ( @@ -164,12 +165,20 @@ mkdir -p "${yaml_dir}/conf" > conf/gremlin-server.yaml [[ "$(get_yaml_authenticator)" == "com.example.FlowAuth" ]] +# align_auth_config must refuse an authentication block without a readable +# authenticator: continuing would let enable-auth.sh write the REST side +# alone (REST on StandardAuthenticator, Gremlin on TinkerPop's +# AllowAllAuthenticator default), so the entrypoint stops here instead. printf '%s\n' \ 'authentication:' \ ' authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler' \ > conf/gremlin-server.yaml unset AUTHENTICATOR_CLASS - align_auth_config + if align_auth_config; then + echo "align_auth_config must refuse an authentication block" \ + "without a readable authenticator" >&2 + exit 1 + fi [[ -z "${AUTHENTICATOR_CLASS:-}" ]] [[ ! -s "${REST_SERVER_CONF}" ]] @@ -181,6 +190,43 @@ mkdir -p "${yaml_dir}/conf" grep -q '^auth\.authenticator=com\.example\.YamlAuth$' "${REST_SERVER_CONF}" ) +# The refusal above is what keeps enable-auth.sh from writing one side: +# against the same ambiguous layout, enable-auth.sh on its own writes only +# the REST file (its yaml guard already sees an `authentication:` line), +# leaving REST on StandardAuthenticator and Gremlin on TinkerPop's +# AllowAllAuthenticator default. The entrypoint never lets it run there +# because align_auth_config fails first under set -e. +onesided_dir="${test_dir}/yaml-onesided" +mkdir -p "${onesided_dir}/bin" "${onesided_dir}/conf/graphs" +cp "$(cd "$(dirname "${BASH_SOURCE[0]}")/../../src/assembly/static/bin" && pwd)/enable-auth.sh" \ + "${onesided_dir}/bin/enable-auth.sh" +chmod +x "${onesided_dir}/bin/enable-auth.sh" +( + cd "${onesided_dir}" || exit 1 + REST_SERVER_CONF="./conf/rest-server.properties" + : > conf/rest-server.properties + printf '%s\n' \ + 'gremlin.graph=org.apache.hugegraph.HugeFactory' \ + > conf/graphs/hugegraph.properties + printf '%s\n' \ + 'authentication:' \ + ' authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler' \ + > conf/gremlin-server.yaml + unset AUTHENTICATOR_CLASS + if align_auth_config; then + echo "align_auth_config must refuse an authentication block without a readable authenticator" >&2 + exit 1 + fi + ./bin/enable-auth.sh + grep -q '^auth\.authenticator=org\.apache\.hugegraph\.auth\.StandardAuthenticator$' \ + conf/rest-server.properties + grep -q 'HugeFactoryAuthProxy' conf/graphs/hugegraph.properties + if grep -Eq '^[[:blank:]]*authenticator[[:blank:]]*:' conf/gremlin-server.yaml; then + echo "enable-auth.sh must not add an authenticator to the yaml block" >&2 + exit 1 + fi +) + # CRLF (Windows-saved) configs parse the way java.util.Properties reads # them: one trailing CR is a line terminator, not part of the value, and # a backslash before CRLF still continues the value onto the next line. From b93b52e5756ae5708f09c421a4527b70a87cfc72 Mon Sep 17 00:00:00 2001 From: Adarsh Mishra <122873385+Adarsh-Me@users.noreply.github.com> Date: Tue, 22 Sep 2026 20:56:19 +0530 Subject: [PATCH 06/11] fix(docker): close the remaining auth bootstrap review gaps Four review findings on the auth bootstrap, each with a case that fails without the change: - enable-auth.sh: the `sed -i '$a\...'` appends were silent no-ops on a file with no lines, so an empty mounted config received neither `auth.authenticator` nor the yaml `authentication:` block while the entrypoint had already applied PASSWORD and init-store had run in auth mode. Append with `>>`, closing a missing trailing newline first. This is the failure that red `docker-build (hugegraph-server/Dockerfile)` reports on run 35101931936. - props.awk: `cat tmp > file` truncates the destination before cat writes, so a mid-copy failure (ENOSPC, EIO) left a half-written config on disk rather than the previous content. Snapshot the original under umask 077 first, restore it when the copy fails, and only drop both staging files once the copy has succeeded. - docker-entrypoint.sh: get_yaml_authenticator opened its block on `authentication:` and never closed it, so an `authenticator:` belonging to any later mapping was read as Gremlin's and then written into the REST config. Track the key's indentation and end the block at the next key at or left of it. - docker-entrypoint.sh: the both-empty branch assigned the default authenticator unconditionally, discarding an operator-supplied AUTHENTICATOR_CLASS before enable-auth.sh could use it. It now only fills the value in when unset. --- .../docker/docker-entrypoint.sh | 16 +- .../hugegraph-dist/docker/props.awk | 34 +++- .../docker/test/test-docker-entrypoint.sh | 171 ++++++++++++++++++ .../src/assembly/static/bin/enable-auth.sh | 32 +++- 4 files changed, 238 insertions(+), 15 deletions(-) diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh index 63f2c9cd07..fde8d253e0 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh @@ -128,6 +128,7 @@ get_yaml_authenticator() { /^[ \t]*#/ { next } /^[ \t]*authentication[ \t]*:/ { inblk = 1 + indent = match($0, /[^ \t]/) line = $0 sub(/^[ \t]*authentication[ \t]*:[ \t]*/, "", line) if (match(line, /authenticator[ \t]*:/)) { @@ -136,6 +137,15 @@ get_yaml_authenticator() { } next } + # A blank line does not close a YAML mapping. + /^[ \t\r]*$/ { next } + # The authenticator has to belong to the authentication mapping: + # any key at or left of that key is a sibling, so the block is + # over. Without this, the first `authenticator:` anywhere below + # `authentication:` is taken as the Gremlin one, which lets a + # later top-level mapping carrying its own authenticator decide + # the REST side too. + inblk && match($0, /[^ \t]/) <= indent { inblk = 0 } inblk && /^[ \t]+authenticator[ \t]*:/ { line = $0 sub(/^[ \t]*authenticator[ \t]*:[ \t]*/, "", line) @@ -187,7 +197,11 @@ align_auth_config() { return fi if [[ -z "${rest_auth}" && -z "${yaml_auth}" ]]; then - export AUTHENTICATOR_CLASS="org.apache.hugegraph.auth.StandardAuthenticator" + # Only fill in a default: an operator-supplied AUTHENTICATOR_CLASS + # is the intent for a config that names no authenticator yet, and + # assigning here would turn it back into StandardAuthenticator + # before enable-auth.sh ever saw it. + export AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}" elif [[ -n "${yaml_auth}" ]]; then set_prop "auth.authenticator" "${yaml_auth}" "${REST_SERVER_CONF}" else diff --git a/hugegraph-server/hugegraph-dist/docker/props.awk b/hugegraph-server/hugegraph-dist/docker/props.awk index 621cd480a8..1680f368c7 100644 --- a/hugegraph-server/hugegraph-dist/docker/props.awk +++ b/hugegraph-server/hugegraph-dist/docker/props.awk @@ -196,7 +196,7 @@ function props_load(file, raw, rc, nl, stripped, next_raw, start, logical) { } } -function props_set(file, key, enc_val, tmp, cmd, b, first, ln) { +function props_set(file, key, enc_val, tmp, bak, cmd, b, first, ln, msg) { props_load(file) first = 0 for (b = 1; b <= NBLOCK; b++) { @@ -233,13 +233,33 @@ function props_set(file, key, enc_val, tmp, cmd, b, first, ln) { # umask-world-readable), turn a symlinked config into a regular file, # and fail with EBUSY on a config bind-mounted as a single file — the # mounted case this path exists for. The copy keeps the inode, mode, - # symlink and mount point, and since the temp file is fully written - # before the original is truncated, a failed copy still leaves the - # previous content on disk. + # symlink and mount point. + # + # The copy itself is not atomic and the shell's `>` truncates the + # destination before cat writes a byte, so an ENOSPC or I/O error + # mid-copy used to leave a truncated config on disk — a truncated + # rest-server.properties loses `auth.authenticator` and boots the + # server with authentication off. Snapshot the original first (under + # umask 077 so a backup of a 0644 mounted config never ends up more + # permissive than it started, and chmodded in case a crashed run left + # one behind) and put it back when the copy fails. system("chmod 600 -- " shquote(tmp)) - cmd = "cat -- " shquote(tmp) " > " shquote(file) " && rm -f -- " shquote(tmp) - if (system(cmd) != 0) - die("cannot copy " tmp " back over " file) + bak = file ".bak" + cmd = "umask 077 && cp -- " shquote(file) " " shquote(bak) + if (system(cmd " && chmod 600 -- " shquote(bak)) != 0) + die("cannot back up " file " before the copy-back") + cmd = "cat -- " shquote(tmp) " > " shquote(file) + if (system(cmd) != 0) { + # Best effort: the destination is already damaged, so restoring it + # from the snapshot comes first, and the temp file is kept for an + # operator who wants to inspect what was being written. + msg = "cannot copy " tmp " over " file + cmd = "cat -- " shquote(bak) " > " shquote(file) + if (system(cmd) == 0) die(msg "; the previous content is restored") + die(msg "; " file " is damaged, previous content is in " bak) + } + if (system("rm -f -- " shquote(tmp) " " shquote(bak)) != 0) + die("cannot remove " tmp " and " bak " after the copy-back") } function props_get(file, key, b) { 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 012cf8907a..3b225d30d7 100644 --- a/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh @@ -283,6 +283,7 @@ set_prop "init_store.enabled" "true" "${mode_file}" grep -q '^init_store\.enabled=true$' "${mode_file}" grep -q '^unrelated=true$' "${mode_file}" [[ ! -e "${mode_file}.tmp" ]] +[[ ! -e "${mode_file}.bak" ]] target_file="${test_dir}/config-target" link_file="${test_dir}/config-link" @@ -291,3 +292,173 @@ ln -s "${target_file}" "${link_file}" set_prop "init_store.enabled" "true" "${link_file}" [[ -L "${link_file}" ]] grep -q '^init_store\.enabled=true$' "${target_file}" + +# An `authenticator:` below a *sibling* mapping is not the Gremlin one. +# `get_yaml_authenticator` opens its block on `authentication:` and has to +# close it again on the next key at the same indentation, or the yaml below +# reports com.example.TlsOnly — and align_auth_config then writes that +# class into rest-server.properties, so REST authenticates with a class the +# operator only ever mentioned to an unrelated mapping. +scope_dir="${test_dir}/yaml-scope" +mkdir -p "${scope_dir}/conf" +( + cd "${scope_dir}" || exit 1 + + printf '%s\n' \ + 'authentication:' \ + ' config: {tokens: conf/rest-server.properties}' \ + 'ssl:' \ + ' authenticator: com.example.TlsOnly' \ + > conf/gremlin-server.yaml + [[ -z "$(get_yaml_authenticator)" ]] + + # The block's own authenticator is still found when a sibling follows + # it, and one deeper than the key is still inside it. + printf '%s\n' \ + 'authentication:' \ + ' authenticator: com.example.GremlinAuth' \ + ' authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler' \ + 'ssl:' \ + ' authenticator: com.example.TlsOnly' \ + > conf/gremlin-server.yaml + [[ "$(get_yaml_authenticator)" == "com.example.GremlinAuth" ]] + + # A blank line does not close a YAML mapping, and neither does a + # comment — including one that names an authenticator. + printf '%s\n' \ + 'authentication:' \ + '' \ + '# authenticator: com.example.CommentedAuth' \ + ' authenticator: com.example.BlankLineAuth' \ + > conf/gremlin-server.yaml + [[ "$(get_yaml_authenticator)" == "com.example.BlankLineAuth" ]] + + # Same indentation as the key means a sibling, not a member: the last + # case a mounted file is likely to get wrong, because a two-space + # `authentication:` under a top-level key is how some deployments + # indent the whole block. + printf '%s\n' \ + ' authentication:' \ + ' authenticator: com.example.IndentedAuth' \ + ' ssl:' \ + ' authenticator: com.example.TlsOnly' \ + > conf/gremlin-server.yaml + [[ "$(get_yaml_authenticator)" == "com.example.IndentedAuth" ]] +) + +# Both sides silent means "bootstrap authentication", but an operator who +# passed AUTHENTICATOR_CLASS named the class they want. The default may +# fill that in, it may not overwrite it: enable-auth.sh appends the value +# it is given, so overwriting here put StandardAuthenticator into a +# deployment that asked for something else. +class_dir="${test_dir}/authenticator-class" +mkdir -p "${class_dir}/conf" +( + cd "${class_dir}" || exit 1 + REST_SERVER_CONF="./conf/rest-server.properties" + : > "${REST_SERVER_CONF}" + printf '%s\n' 'restserver.url=http://0.0.0.0:8080' > conf/gremlin-server.yaml + + AUTHENTICATOR_CLASS=com.example.OperatorAuth + export AUTHENTICATOR_CLASS + align_auth_config + [[ "${AUTHENTICATOR_CLASS}" == "com.example.OperatorAuth" ]] + + unset AUTHENTICATOR_CLASS + align_auth_config + [[ "${AUTHENTICATOR_CLASS}" == \ + "org.apache.hugegraph.auth.StandardAuthenticator" ]] +) + +# An empty mounted config still gets its definitions. GNU sed's `$` +# address never matches when the file has no lines, so enable-auth.sh's +# `sed -i '$a\...'` appends were silent no-ops on an empty +# rest-server.properties and an empty gremlin-server.yaml: the +# entrypoint had already written auth.admin_pa and init-store had run in +# auth mode, yet neither server was told to authenticate at all. +empty_dir="${test_dir}/empty-config" +mkdir -p "${empty_dir}/bin" "${empty_dir}/conf/graphs" +cp "$(cd "$(dirname "${BASH_SOURCE[0]}")/../../src/assembly/static/bin" && pwd)/enable-auth.sh" \ + "${empty_dir}/bin/enable-auth.sh" +chmod +x "${empty_dir}/bin/enable-auth.sh" +( + cd "${empty_dir}" || exit 1 + : > conf/rest-server.properties + : > conf/gremlin-server.yaml + printf '%s\n' 'gremlin.graph=org.apache.hugegraph.HugeFactory' \ + > conf/graphs/hugegraph.properties + unset AUTHENTICATOR_CLASS + ./bin/enable-auth.sh + grep -q '^auth\.authenticator=org\.apache\.hugegraph\.auth\.StandardAuthenticator$' \ + conf/rest-server.properties + grep -q '^auth\.graph_store=hugegraph$' conf/rest-server.properties + grep -q '^authentication: {$' conf/gremlin-server.yaml + grep -q '^ authenticator: org\.apache\.hugegraph\.auth\.StandardAuthenticator,$' \ + conf/gremlin-server.yaml + grep -q '^ config: {tokens: conf/rest-server\.properties}$' \ + conf/gremlin-server.yaml + grep -q '^}' conf/gremlin-server.yaml + grep -q 'HugeFactoryAuthProxy' conf/graphs/hugegraph.properties + # Idempotent: a second run adds nothing to what the first one wrote. + wc -l < conf/gremlin-server.yaml > "${test_dir}/empty-yaml-count" + ./bin/enable-auth.sh + [[ "$(wc -l < conf/gremlin-server.yaml)" == \ + "$(cat "${test_dir}/empty-yaml-count")" ]] + + # A config whose last line has no terminator still gets a line of its + # own; `sed -i '$a'` closed that terminator for us. + printf 'restserver.url=http://127.0.0.1:8080' > conf/rest-server.properties + ./bin/enable-auth.sh + grep -q '^auth\.authenticator=' conf/rest-server.properties + grep -q '^restserver\.url=http://127\.0\.0\.1:8080$' conf/rest-server.properties +) + +# A copy-back that fails part way must not leave a truncated config. The +# shell's `>` truncates the destination before cat writes a byte, so +# props.awk snapshots the original first and puts it back. The snapshot +# `cat` is replaced through PATH to fail the copy the way ENOSPC would: +# stdout here *is* the already-truncated destination, so a few bytes and a +# non-zero exit is exactly a half-written config. +failbin="${test_dir}/fakebin" +mkdir -p "${failbin}" +real_cat="$(command -v cat)" +printf '%s\n' \ + '#!/bin/sh' \ + 'case "$*" in' \ + ' *.tmp) printf "auth.authenticator=par"; exit 1 ;;' \ + 'esac' \ + 'exec "${FAKE_CAT_REAL}" "$@"' \ + > "${failbin}/cat" +chmod +x "${failbin}/cat" +rb_file="${test_dir}/config-rollback" +rb_expect="${test_dir}/config-rollback.expected" +printf '%s\n' \ + 'auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' \ + 'auth.token_secret=s3cr3t' \ + 'unrelated=true' > "${rb_file}" +cp -p "${rb_file}" "${rb_expect}" +( + PATH="${failbin}:${PATH}" + FAKE_CAT_REAL="${real_cat}" + export PATH FAKE_CAT_REAL + if set_prop 'auth.authenticator' 'com.example.HalfWritten' "${rb_file}"; then + echo "set_prop must fail when the copy-back fails" >&2 + exit 1 + fi +) 2>/dev/null +cmp -s "${rb_file}" "${rb_expect}" || { + echo "a failed copy-back must leave the previous content in place" >&2 + exit 1 +} +# Both staging files survive on purpose: the temp file is what was being +# written, and the snapshot is the operator's way back. +[[ -e "${rb_file}.tmp" ]] +[[ -e "${rb_file}.bak" ]] +# Once the condition clears the same set goes through, and leaves nothing +# behind. +set_prop 'auth.authenticator' 'com.example.HalfWritten' "${rb_file}" +grep -q '^auth\.authenticator=com\.example\.HalfWritten$' "${rb_file}" +grep -q '^auth\.token_secret=s3cr3t$' "${rb_file}" +grep -q '^unrelated=true$' "${rb_file}" +[[ ! -e "${rb_file}.tmp" ]] +[[ ! -e "${rb_file}.bak" ]] 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 8524894f26..8737d20088 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 @@ -55,22 +55,40 @@ fi # resolved in opposite directions, leaving Gremlin and REST on different # authenticators. +# Appended with `>>` rather than `sed -i '$a\...'`: GNU sed's `$` address +# never matches when the file has no lines, so on an empty mounted config +# every append below silently did nothing. Neither the REST +# `auth.authenticator` nor the yaml `authentication:` block was written, +# while the entrypoint had already applied PASSWORD and init-store had run +# in auth mode — the servers then came up unauthenticated with no error. +# `sed -i '$a'` also closed the previous last line for us, which `>>` does +# not, so a file without a trailing newline gets one first. +append_lines() { + local file="$1" + shift + if [[ -s "${file}" && -n "$(tail -c 1 "${file}")" ]]; then + printf '\n' >> "${file}" + fi + printf '%s\n' "$@" >> "${file}" +} + AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}" 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,' \ - -e '$a\ config: {tokens: conf/rest-server.properties}' \ - -e '$a\}' ${CONF}/${GREMLIN_SERVER_CONF} + append_lines "${CONF}/${GREMLIN_SERVER_CONF}" \ + 'authentication: {' \ + " authenticator: ${AUTHENTICATOR_CLASS}," \ + ' authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler,' \ + ' config: {tokens: conf/rest-server.properties}' \ + '}' fi 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} + append_lines "${CONF}/${REST_SERVER_CONF}" "auth.authenticator=${AUTHENTICATOR_CLASS}" fi 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} + append_lines "${CONF}/${REST_SERVER_CONF}" 'auth.graph_store=hugegraph' fi # GNU grep reads \r in a pattern as the letter r, so the carriage return a From bf8303294ba962224662a0b21da568329ef8459a Mon Sep 17 00:00:00 2001 From: Adarsh Mishra <122873385+Adarsh-Me@users.noreply.github.com> Date: Tue, 22 Sep 2026 21:03:20 +0530 Subject: [PATCH 07/11] test(docker): cover the rollback branch where restoring also fails The injected `cat` can fail the copy-back and the restore at once, which is the only case where props.awk cannot repair the config. Assert the operator is pointed at the snapshot, that the snapshot is a byte-for-byte copy of what was there before, and that both staging files are left behind. --- .../docker/test/test-docker-entrypoint.sh | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) 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 3b225d30d7..b89dc43d14 100644 --- a/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh @@ -426,6 +426,7 @@ printf '%s\n' \ '#!/bin/sh' \ 'case "$*" in' \ ' *.tmp) printf "auth.authenticator=par"; exit 1 ;;' \ + ' *.bak) [ -n "${FAKE_BAK_FAIL:-}" ] && exit 1' \ 'esac' \ 'exec "${FAKE_CAT_REAL}" "$@"' \ > "${failbin}/cat" @@ -462,3 +463,29 @@ grep -q '^auth\.token_secret=s3cr3t$' "${rb_file}" grep -q '^unrelated=true$' "${rb_file}" [[ ! -e "${rb_file}.tmp" ]] [[ ! -e "${rb_file}.bak" ]] +# When the restore fails too there is nothing left to do but say so and +# point at the snapshot, because that snapshot is the only copy of a +# working config the operator has. +printf '%s\n' \ + 'auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' \ + 'auth.token_secret=s3cr3t' \ + 'unrelated=true' > "${rb_file}" +rb_out=$( + PATH="${failbin}:${PATH}" + FAKE_CAT_REAL="${real_cat}" + FAKE_BAK_FAIL=1 + export PATH FAKE_CAT_REAL FAKE_BAK_FAIL + set_prop 'auth.authenticator' 'com.example.HalfWritten' "${rb_file}" 2>&1 +) || true +[[ "${rb_out}" == *"${rb_file}.bak"* ]] || { + echo "props.awk must name the snapshot when the restore also fails" >&2 + exit 1 +} +# The damaged config keeps whatever the aborted copy left, and the +# snapshot still holds the last known good content. +[[ -e "${rb_file}.bak" ]] +[[ -e "${rb_file}.tmp" ]] +cmp -s "${rb_file}.bak" "${rb_expect}" || { + echo "the snapshot must be a byte-for-byte copy of the original" >&2 + exit 1 +} From b8801a650633639e0ee387171cdb4d328af8e0a1 Mon Sep 17 00:00:00 2001 From: Adarsh Date: Wed, 23 Sep 2026 14:08:44 +0530 Subject: [PATCH 08/11] fix(docker): decide authentication by side, not by class Takes the review's simplification: the entrypoint no longer parses which authenticator gremlin-server.yaml names, so get_yaml_authenticator, the get-decoded mode in props.awk and the class comparison in align_auth_config all go away. What is kept is the guarantee those served - REST and Gremlin never end up with authentication on one side only - by refusing every one-sided layout instead of completing it. The refusal is kept honest by a three-state read of the yaml (none / named / nameless). Treating a mapping that names no authenticator as "no yaml side" would pass a REST-only config straight through to enable-auth.sh, whose guard only looks for the presence of the mapping and so would write the REST file alone: REST on StandardAuthenticator, Gremlin on TinkerPop's AllowAllAuthenticator default. That is the fail-open this PR exists to close. Also from the review: - the authentication key must start at column 0, so a mapping nested under an unrelated feature no longer decides the REST side. This drops the case the previous test asserted for an indented `authentication:`; a top-level key is what gremlin-server.yaml actually uses. - props_set refuses a value whose encoded form ends in an odd number of backslashes. Written where it is no longer the last line it swallows the line after it, and commons-configuration2 reads such a pair back as no property at all, so the entrypoint would publish a secret no server sees. - the post-startup backend read goes through props.awk, so a mounted `backend : hstore` no longer skips the partition-wait check silently. Net -56 lines of shell and awk, +105 of tests. Every new case was checked by reverting its fix: the backslash refusal, the column-0 requirement, the nameless refusal and the refusal path itself each turn the suite red on their own. Suite green here apart from the four assertions this host cannot execute. --- .../docker/docker-entrypoint.sh | 185 ++++------ .../hugegraph-dist/docker/props.awk | 27 +- .../docker/test/test-docker-entrypoint.sh | 319 ++++++++++++------ 3 files changed, 290 insertions(+), 241 deletions(-) diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh index fde8d253e0..1f238152e9 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh @@ -79,136 +79,77 @@ get_prop_encoded() { awk -f "${PROPS_AWK}" /dev/null } -# Decoded read: unescapes the on-disk value the way java.util.Properties -# does, so it compares equal with the snakeyaml-decoded scalar from -# get_yaml_authenticator. The raw get_prop_encoded mode stays for the -# secret round trip, which must replay backslashes byte-for-byte. -get_prop() { - local key="$1" file="$2" - - PROPS_MODE=get-decoded PROPS_KEY="${key}" PROPS_FILE="${file}" \ - awk -f "${PROPS_AWK}" /dev/null -} - -# First uncommented `authenticator:` inside the gremlin-server.yaml -# 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() { +# What the top-level authentication mapping of gremlin-server.yaml says about +# authentication, as one of three states: +# +# none no such mapping +# named the mapping carries an authenticator +# nameless the mapping exists but names no authenticator +# +# Only presence is asked for, never the class: the entrypoint does not copy a +# value between the two files any more, so quotes, inline comments and flow +# mappings stay snakeyaml's business instead of becoming a parser here. The +# key must start at column 0 — an `authentication:` nested under another +# mapping belongs to that feature, not to the Gremlin server, and reading it as +# the Gremlin one would let an unrelated class decide whether REST is +# authenticated while Gremlin stayed on TinkerPop's AllowAllAuthenticator. +yaml_auth_state() { local yaml="./conf/gremlin-server.yaml" - [[ -f "${yaml}" ]] || return 0 + [[ -f "${yaml}" ]] || { echo "none"; 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]*:/ { + /^authentication[ \t]*:/ { inblk = 1 - indent = match($0, /[^ \t]/) - line = $0 - sub(/^[ \t]*authentication[ \t]*:[ \t]*/, "", line) - if (match(line, /authenticator[ \t]*:/)) { - print scalar(substr(line, RSTART + RLENGTH)) - exit - } + have = 1 + # A flow mapping keeps the authenticator on the same line as the + # key, so it has to count there too; missing it would report a + # configured mapping as nameless and refuse a valid deployment. + if (match($0, /authenticator[ \t]*:/)) { named = 1; exit } next } - # A blank line does not close a YAML mapping. - /^[ \t\r]*$/ { next } - # The authenticator has to belong to the authentication mapping: - # any key at or left of that key is a sibling, so the block is - # over. Without this, the first `authenticator:` anywhere below - # `authentication:` is taken as the Gremlin one, which lets a - # later top-level mapping carrying its own authenticator decide - # the REST side too. - inblk && match($0, /[^ \t]/) <= indent { inblk = 0 } - inblk && /^[ \t]+authenticator[ \t]*:/ { - line = $0 - sub(/^[ \t]*authenticator[ \t]*:[ \t]*/, "", line) - print scalar(line) - exit + # Any other column-0 key ends the mapping. A blank or whitespace-only + # line does not, because YAML does not close a mapping on an empty line. + inblk && /^[^ \t]/ { inblk = 0 } + inblk && /^[ \t]+authenticator[ \t]*:/ { named = 1; exit } + END { + if (named) print "named" + else if (have) print "nameless" + else print "none" } ' "${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 -# 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 "auth.authenticator" "${REST_SERVER_CONF}") - yaml_auth=$(get_yaml_authenticator) - if [[ -z "${yaml_auth}" ]] && has_yaml_authentication_block; then - # Refuse instead of bootstrapping one side: enable-auth.sh runs right - # after align and only touches the REST side, so continuing would put - # REST on StandardAuthenticator while Gremlin stays on TinkerPop's - # AllowAllAuthenticator default. Failing fast (rather than skipping - # enable-auth.sh) keeps a PASSWORD deployment from starting with - # authentication silently half-applied. - log "ERROR: gremlin-server.yaml carries an authentication block" \ - "without a readable authenticator; refusing to bootstrap" \ - "authentication one-sided. Add an 'authenticator:' entry to" \ - "the block or remove the block, then restart." +# Authentication has to be configured on both sides or on neither. A mounted +# config carrying only one is refused rather than completed: the entrypoint +# cannot know which class the operator means, and finishing the other side from +# a guessed default is how Gremlin ends up on AllowAllAuthenticator while REST +# enforces StandardAuthenticator. A mapping that names no authenticator is +# refused by itself, because enable-auth.sh guards on the presence of that +# mapping and would otherwise write only the REST side. +check_auth_sides() { + local rest=0 yaml=0 state + + state=$(yaml_auth_state) + if [[ "${state}" == "nameless" ]]; then + log "ERROR: gremlin-server.yaml carries a top-level authentication" \ + "mapping that names no authenticator; add an authenticator entry" \ + "to it or remove the mapping, then restart." return 1 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" - return + if [[ -n "$(get_prop_encoded "auth.authenticator" "${REST_SERVER_CONF}")" ]]; then + rest=1 fi - if [[ -z "${rest_auth}" && -z "${yaml_auth}" ]]; then - # Only fill in a default: an operator-supplied AUTHENTICATOR_CLASS - # is the intent for a config that names no authenticator yet, and - # assigning here would turn it back into StandardAuthenticator - # before enable-auth.sh ever saw it. - export AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}" - elif [[ -n "${yaml_auth}" ]]; then - set_prop "auth.authenticator" "${yaml_auth}" "${REST_SERVER_CONF}" - else - export AUTHENTICATOR_CLASS="${rest_auth}" + if [[ "${state}" == "named" ]]; then + yaml=1 + fi + if (( rest == yaml )); then + return 0 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. + log "ERROR: authentication is configured in only one of" \ + "rest-server.properties (auth.authenticator) and" \ + "gremlin-server.yaml (authentication.authenticator);" \ + "configure both or neither, then restart." + return 1 } migrate_env() { @@ -280,9 +221,9 @@ elif [[ -n "${AUTH_TOKEN_SECRET_ENCODED}" ]]; then fi if [[ -n "${PASSWORD:-}" ]]; then set_prop "auth.admin_pa" "${PASSWORD}" "${REST_SERVER_CONF}" - # A refusal inside align_auth_config exits the entrypoint under set -e, - # so enable-auth.sh can never run one-sided after it. - align_auth_config + # A refusal here exits the entrypoint under set -e, so enable-auth.sh can + # never run one-sided after it. + check_auth_sides # 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 @@ -356,7 +297,11 @@ fi ./bin/start-hugegraph.sh -j "${JAVA_OPTS:-}" -t 120 # Post-startup cluster stabilization check (hstore only — rocksdb has no partitions) -ACTUAL_BACKEND=$(grep -E '^[[:space:]]*backend[[:space:]]*=' "${GRAPH_CONF}" | head -n 1 | sed 's/.*=//' | tr -d '[:space:]' || true) +# Read through props.awk so a mounted config using the `:` or bare-whitespace +# separator is seen at all, and first-definition-wins matches HugeConfig; the +# grep this replaces only ever accepted `=`. Trailing whitespace is dropped +# here rather than in the reader, which reports the on-disk bytes verbatim. +ACTUAL_BACKEND=$(get_prop_encoded "backend" "${GRAPH_CONF}" | tr -d '[:space:]' || true) if [[ "${ACTUAL_BACKEND}" == "hstore" ]]; then STORE_REST="${STORE_REST:-store:8520}" export STORE_REST diff --git a/hugegraph-server/hugegraph-dist/docker/props.awk b/hugegraph-server/hugegraph-dist/docker/props.awk index 1680f368c7..1dfb7ab8ae 100644 --- a/hugegraph-server/hugegraph-dist/docker/props.awk +++ b/hugegraph-server/hugegraph-dist/docker/props.awk @@ -196,8 +196,19 @@ function props_load(file, raw, rc, nl, stripped, next_raw, start, logical) { } } -function props_set(file, key, enc_val, tmp, bak, cmd, b, first, ln, msg) { +function props_set(file, key, enc_val, tmp, bak, cmd, b, first, ln, msg, nbs) { props_load(file) + # A value whose encoded form ends in an odd number of backslashes would + # turn the line written after it into a continuation of that value. + # Measured against commons-configuration2 (what HugeConfig extends), the + # same input read back yields no property at all, so a secret written this + # way would never reach the server that is supposed to authenticate with + # it; the entrypoint has to refuse instead of guessing a target. + nbs = 0 + while (nbs < length(enc_val) && substr(enc_val, length(enc_val) - nbs, 1) == "\\") + nbs++ + if (nbs % 2 == 1) + die("refusing to write " key ": the encoded value ends in an odd number of backslashes") first = 0 for (b = 1; b <= NBLOCK; b++) { if (BTYPE[b] == "entry" && BKEY[b] == key) { @@ -272,16 +283,6 @@ function props_get(file, key, b) { } } -function props_get_decoded(file, key, b) { - props_load(file) - for (b = 1; b <= NBLOCK; b++) { - if (BTYPE[b] == "entry" && BKEY[b] == key) { - print unescape(BVAL[b]) - return - } - } -} - BEGIN { mode = ENVIRON["PROPS_MODE"] key = ENVIRON["PROPS_KEY"] @@ -290,11 +291,9 @@ BEGIN { die("PROPS_FILE and PROPS_KEY must be set") if (mode == "get") { props_get(file, key) - } else if (mode == "get-decoded") { - props_get_decoded(file, key) } else if (mode == "set") { props_set(file, key, ENVIRON["PROPS_VALUE_ENCODED"]) } else { - die("PROPS_MODE must be get, get-decoded or set") + 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 b89dc43d14..536facfc47 100644 --- a/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh @@ -27,8 +27,8 @@ trap 'rm -rf "${test_dir}"' EXIT # 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_prop \ - get_yaml_authenticator has_yaml_authentication_block align_auth_config; do +for fn in encode_prop_value set_prop_encoded set_prop get_prop_encoded \ + yaml_auth_state check_auth_sides; do eval "$(awk -v fn="${fn}" ' index($0, fn "() {") == 1 { capture = 1 } capture { print } @@ -138,56 +138,137 @@ 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 refuses an authentication block without a readable -# authenticator instead of treating it as "no yaml side": exporting the -# default there would override an explicit choice, and continuing would let -# enable-auth.sh write the REST side alone. +# yaml_auth_state reports whether the top-level authentication mapping names +# an authenticator, without ever reading the class: quoted scalars and inline +# comments still count as naming one, a flow mapping on the key line counts, a +# mapping with no authenticator is "nameless", and an `authentication:` nested +# under some other key is not the Gremlin mapping at all. 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}" + state_file="conf/gremlin-server.yaml" + + want_state() { + if [[ "$1" != "$2" ]]; then + echo "expected yaml state '$1', got '$2'" >&2 + exit 1 + fi + } + + printf '%s\n' 'host: 0.0.0.0' > "${state_file}" + want_state none "$(yaml_auth_state)" + + printf '%s\n' \ + 'authentication:' \ + ' authenticator: com.example.MyAuth' \ + > "${state_file}" + want_state named "$(yaml_auth_state)" 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" ]] + > "${state_file}" + want_state named "$(yaml_auth_state)" 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" ]] + > "${state_file}" + want_state named "$(yaml_auth_state)" -# align_auth_config must refuse an authentication block without a readable -# authenticator: continuing would let enable-auth.sh write the REST side -# alone (REST on StandardAuthenticator, Gremlin on TinkerPop's -# AllowAllAuthenticator default), so the entrypoint stops here instead. printf '%s\n' \ 'authentication:' \ ' authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler' \ - > conf/gremlin-server.yaml - unset AUTHENTICATOR_CLASS - if align_auth_config; then - echo "align_auth_config must refuse an authentication block" \ - "without a readable authenticator" >&2 - exit 1 - fi - [[ -z "${AUTHENTICATOR_CLASS:-}" ]] - [[ ! -s "${REST_SERVER_CONF}" ]] + > "${state_file}" + want_state nameless "$(yaml_auth_state)" + + # The nested mapping belongs to someFeature, not to the Gremlin server. + # Reading it as the Gremlin one would let com.example.Nested authenticate + # REST while Gremlin stayed on TinkerPop's AllowAllAuthenticator default. + printf '%s\n' \ + 'someFeature:' \ + ' authentication:' \ + ' authenticator: com.example.Nested' \ + > "${state_file}" + want_state none "$(yaml_auth_state)" + + # An authenticator that only appears after the block ends is a sibling's. + printf '%s\n' \ + 'authentication:' \ + ' tokens: conf/rest-server.properties' \ + 'other:' \ + ' authenticator: com.example.Other' \ + > "${state_file}" + want_state nameless "$(yaml_auth_state)" + # A blank line does not close a YAML mapping. printf '%s\n' \ 'authentication:' \ - ' authenticator: com.example.YamlAuth' \ + ' tokens: conf/rest-server.properties' \ + '' \ + ' authenticator: com.example.Later' \ + > "${state_file}" + want_state named "$(yaml_auth_state)" + + rm -f "${state_file}" + want_state none "$(yaml_auth_state)" +) + +# check_auth_sides keeps the guarantee the class parsing used to serve: REST and +# Gremlin never end up with authentication on one side only. Neither and both +# pass; one side, or a mapping that names no authenticator, stops the boot. +sides_dir="${test_dir}/sides" +mkdir -p "${sides_dir}/conf" +( + cd "${sides_dir}" || exit 1 + REST_SERVER_CONF="./conf/rest-server.properties" + + must_refuse() { + if check_auth_sides; then + echo "check_auth_sides must refuse: $1" >&2 + exit 1 + fi + } + + printf '%s\n' 'host: 0.0.0.0' > conf/gremlin-server.yaml + : > "${REST_SERVER_CONF}" + check_auth_sides + + # Both sides configured, different classes: untouched. enable-auth.sh's + # per-file guards then make its appends no-ops, so nothing here has to + # know which class either side names. + printf '%s\n' 'auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' \ + > "${REST_SERVER_CONF}" + printf '%s\n' 'authentication:' ' authenticator: com.example.OtherAuth' \ > conf/gremlin-server.yaml - align_auth_config - grep -q '^auth\.authenticator=com\.example\.YamlAuth$' "${REST_SERVER_CONF}" + check_auth_sides + grep -Eq '^[[:blank:]]*auth[\\]?\.authenticator[[:blank:]]*([:=]|[[:blank:]])com\.example\.OtherAuth' \ + "${REST_SERVER_CONF}" && { + echo "check_auth_sides must not copy a class into rest-server.properties" >&2 + exit 1 + } + + # One side only. + printf '%s\n' 'auth.authenticator=com.example.MyAuth' > "${REST_SERVER_CONF}" + printf '%s\n' 'host: 0.0.0.0' > conf/gremlin-server.yaml + must_refuse "rest-server.properties names an authenticator and the yaml does not" + + : > "${REST_SERVER_CONF}" + printf '%s\n' 'authentication:' ' authenticator: com.example.YamlAuth' \ + > conf/gremlin-server.yaml + must_refuse "the yaml names an authenticator and rest-server.properties does not" + + # A mapping that names no authenticator is refused even when REST is empty: + # enable-auth.sh guards on the presence of `authentication:`, so it would + # write the REST file alone and leave Gremlin unauthenticated. + printf '%s\n' 'authentication:' \ + ' authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler' \ + > conf/gremlin-server.yaml + : > "${REST_SERVER_CONF}" + must_refuse "the yaml mapping names no authenticator" + printf '%s\n' 'auth.authenticator=com.example.MyAuth' > "${REST_SERVER_CONF}" + must_refuse "the yaml mapping names no authenticator and REST does" ) # The refusal above is what keeps enable-auth.sh from writing one side: @@ -195,7 +276,7 @@ mkdir -p "${yaml_dir}/conf" # the REST file (its yaml guard already sees an `authentication:` line), # leaving REST on StandardAuthenticator and Gremlin on TinkerPop's # AllowAllAuthenticator default. The entrypoint never lets it run there -# because align_auth_config fails first under set -e. +# because check_auth_sides fails first under set -e. onesided_dir="${test_dir}/yaml-onesided" mkdir -p "${onesided_dir}/bin" "${onesided_dir}/conf/graphs" cp "$(cd "$(dirname "${BASH_SOURCE[0]}")/../../src/assembly/static/bin" && pwd)/enable-auth.sh" \ @@ -212,9 +293,8 @@ chmod +x "${onesided_dir}/bin/enable-auth.sh" 'authentication:' \ ' authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler' \ > conf/gremlin-server.yaml - unset AUTHENTICATOR_CLASS - if align_auth_config; then - echo "align_auth_config must refuse an authentication block without a readable authenticator" >&2 + if check_auth_sides; then + echo "check_auth_sides must refuse a yaml mapping without an authenticator" >&2 exit 1 fi ./bin/enable-auth.sh @@ -238,8 +318,6 @@ printf 'unrelated=true\r\n' >> "${crlf_file}" [[ "$(get_prop_encoded 'auth.authenticator' "${crlf_file}")" == \ "org.apache.hugegraph.auth.StandardAuthenticator" ]] [[ "$(get_prop_encoded 'pd.peers' "${crlf_file}")" == "a,b" ]] -[[ "$(get_prop 'auth.authenticator' "${crlf_file}")" == \ - "org.apache.hugegraph.auth.StandardAuthenticator" ]] set_prop 'auth.authenticator' 'com.example.NewAuth' "${crlf_file}" grep -q '^auth\.authenticator=com\.example\.NewAuth$' "${crlf_file}" [[ "$(get_prop_encoded 'pd.peers' "${crlf_file}")" == "a,b" ]] @@ -248,27 +326,26 @@ if ! grep -q $'^unrelated=true\r$' "${crlf_file}"; then exit 1 fi -# An escaped authenticator and a plain yaml scalar name the same class: -# the comparison unescapes first, so no spurious WARN and no skipped -# alignment. -escaped_auth_dir="${test_dir}/yaml-escaped-auth" +# An escaped key is the same key: java.util.Properties unescapes the name, so +# `auth\.authenticator` has to be found by a read or a write of +# `auth.authenticator` instead of being treated as absent and appended beside. +# (Comparing the class across the two files went away with the yaml scalar +# parser, so only the key grammar is left to pin down here.) +escaped_auth_dir="${test_dir}/escaped-auth-key" mkdir -p "${escaped_auth_dir}/conf" ( cd "${escaped_auth_dir}" || exit 1 REST_SERVER_CONF="./conf/rest-server.properties" printf '%s\n' \ - 'auth.authenticator=org.apache.hugegraph.auth\.StandardAuthenticator' \ + 'auth\.authenticator=com.example.OldAuth' \ + 'unrelated=true' \ > "${REST_SERVER_CONF}" - printf '%s\n' \ - 'authentication:' \ - ' authenticator: org.apache.hugegraph.auth.StandardAuthenticator' \ - > conf/gremlin-server.yaml - unset AUTHENTICATOR_CLASS - align_out=$(align_auth_config 2>&1) - [[ -z "${AUTHENTICATOR_CLASS:-}" ]] - [[ "${align_out}" != *"different authenticators"* ]] - grep -q '^auth\.authenticator=org\.apache\.hugegraph\.auth\.StandardAuthenticator$' \ - "${REST_SERVER_CONF}" + [[ "$(get_prop_encoded 'auth.authenticator' "${REST_SERVER_CONF}")" == \ + "com.example.OldAuth" ]] + set_prop 'auth.authenticator' 'com.example.NewAuth' "${REST_SERVER_CONF}" + assert_line_count 1 'auth[\\]?\.authenticator' "${REST_SERVER_CONF}" + grep -q '^auth\.authenticator=com\.example\.NewAuth$' "${REST_SERVER_CONF}" + assert_line_count 1 '^unrelated=true$' "${REST_SERVER_CONF}" ) # A set must keep the config's inode: a copy-back preserves the file's @@ -293,27 +370,20 @@ set_prop "init_store.enabled" "true" "${link_file}" [[ -L "${link_file}" ]] grep -q '^init_store\.enabled=true$' "${target_file}" -# An `authenticator:` below a *sibling* mapping is not the Gremlin one. -# `get_yaml_authenticator` opens its block on `authentication:` and has to -# close it again on the next key at the same indentation, or the yaml below -# reports com.example.TlsOnly — and align_auth_config then writes that -# class into rest-server.properties, so REST authenticates with a class the -# operator only ever mentioned to an unrelated mapping. +# Two yaml shapes the scoping has to keep getting right: a sibling mapping +# that carries its own authenticator must not hide the block's, and a commented +# authenticator must not count as one. scope_dir="${test_dir}/yaml-scope" mkdir -p "${scope_dir}/conf" ( cd "${scope_dir}" || exit 1 + want_state() { + if [[ "$1" != "$2" ]]; then + echo "expected yaml state '$1', got '$2'" >&2 + exit 1 + fi + } - printf '%s\n' \ - 'authentication:' \ - ' config: {tokens: conf/rest-server.properties}' \ - 'ssl:' \ - ' authenticator: com.example.TlsOnly' \ - > conf/gremlin-server.yaml - [[ -z "$(get_yaml_authenticator)" ]] - - # The block's own authenticator is still found when a sibling follows - # it, and one deeper than the key is still inside it. printf '%s\n' \ 'authentication:' \ ' authenticator: com.example.GremlinAuth' \ @@ -321,53 +391,58 @@ mkdir -p "${scope_dir}/conf" 'ssl:' \ ' authenticator: com.example.TlsOnly' \ > conf/gremlin-server.yaml - [[ "$(get_yaml_authenticator)" == "com.example.GremlinAuth" ]] + want_state named "$(yaml_auth_state)" - # A blank line does not close a YAML mapping, and neither does a - # comment — including one that names an authenticator. printf '%s\n' \ 'authentication:' \ - '' \ '# authenticator: com.example.CommentedAuth' \ - ' authenticator: com.example.BlankLineAuth' \ - > conf/gremlin-server.yaml - [[ "$(get_yaml_authenticator)" == "com.example.BlankLineAuth" ]] - - # Same indentation as the key means a sibling, not a member: the last - # case a mounted file is likely to get wrong, because a two-space - # `authentication:` under a top-level key is how some deployments - # indent the whole block. - printf '%s\n' \ - ' authentication:' \ - ' authenticator: com.example.IndentedAuth' \ - ' ssl:' \ - ' authenticator: com.example.TlsOnly' \ + ' authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler' \ > conf/gremlin-server.yaml - [[ "$(get_yaml_authenticator)" == "com.example.IndentedAuth" ]] + want_state nameless "$(yaml_auth_state)" ) -# Both sides silent means "bootstrap authentication", but an operator who -# passed AUTHENTICATOR_CLASS named the class they want. The default may -# fill that in, it may not overwrite it: enable-auth.sh appends the value -# it is given, so overwriting here put StandardAuthenticator into a -# deployment that asked for something else. +# Both sides silent means "bootstrap authentication", and the class then comes +# from enable-auth.sh: an operator who passed AUTHENTICATOR_CLASS gets the class +# they asked for, and only an unset one falls back to StandardAuthenticator. +# With the entrypoint no longer exporting a class of its own, this is the whole +# of the guarantee, so it is asserted where the default now lives. class_dir="${test_dir}/authenticator-class" -mkdir -p "${class_dir}/conf" ( - cd "${class_dir}" || exit 1 - REST_SERVER_CONF="./conf/rest-server.properties" - : > "${REST_SERVER_CONF}" - printf '%s\n' 'restserver.url=http://0.0.0.0:8080' > conf/gremlin-server.yaml - - AUTHENTICATOR_CLASS=com.example.OperatorAuth - export AUTHENTICATOR_CLASS - align_auth_config - [[ "${AUTHENTICATOR_CLASS}" == "com.example.OperatorAuth" ]] - - unset AUTHENTICATOR_CLASS - align_auth_config - [[ "${AUTHENTICATOR_CLASS}" == \ - "org.apache.hugegraph.auth.StandardAuthenticator" ]] + # A fresh tree per run: enable-auth.sh keeps its own backup of the configs + # it writes, so re-running it over one directory is not a clean case. + run_enable_auth() { + local dir="$1" want="$2" + mkdir -p "${dir}/bin" "${dir}/conf/graphs" + cp "$(cd "$(dirname "${BASH_SOURCE[0]}")/../../src/assembly/static/bin" && pwd)/enable-auth.sh" \ + "${dir}/bin/enable-auth.sh" + chmod +x "${dir}/bin/enable-auth.sh" + printf '%s\n' 'gremlin.graph=org.apache.hugegraph.HugeFactory' \ + > "${dir}/conf/graphs/hugegraph.properties" + : > "${dir}/conf/rest-server.properties" + : > "${dir}/conf/gremlin-server.yaml" + ( + cd "${dir}" || exit 1 + if [[ -n "${want}" ]]; then + AUTHENTICATOR_CLASS="${want}" + export AUTHENTICATOR_CLASS + else + unset AUTHENTICATOR_CLASS + fi + ./bin/enable-auth.sh + ) + } + + run_enable_auth "${class_dir}/operator" "com.example.OperatorAuth" + grep -q '^auth\.authenticator=com\.example\.OperatorAuth$' \ + "${class_dir}/operator/conf/rest-server.properties" + grep -q '^ authenticator: com\.example\.OperatorAuth,$' \ + "${class_dir}/operator/conf/gremlin-server.yaml" + + run_enable_auth "${class_dir}/default" "" + grep -q '^auth\.authenticator=org\.apache\.hugegraph\.auth\.StandardAuthenticator$' \ + "${class_dir}/default/conf/rest-server.properties" + grep -q '^ authenticator: org\.apache\.hugegraph\.auth\.StandardAuthenticator,$' \ + "${class_dir}/default/conf/gremlin-server.yaml" ) # An empty mounted config still gets its definitions. GNU sed's `$` @@ -489,3 +564,33 @@ cmp -s "${rb_file}.bak" "${rb_expect}" || { echo "the snapshot must be a byte-for-byte copy of the original" >&2 exit 1 } + +# A value whose encoded form ends in an odd number of backslashes must not be +# written at all. The entrypoint copies an existing secret between files with +# set_prop_encoded, replaying the raw bytes, and on disk `key=abc\` as the last +# line of a mounted config reads back as no property at all under +# commons-configuration2 (what HugeConfig extends). Written into a file where +# it is no longer last, it turns the following line into a continuation of the +# secret: the server then sees neither the secret nor that property, and the +# entrypoint has published a credential nothing will read. +bs_file="${test_dir}/config-trailing-backslash" +bs_pristine="${test_dir}/config-trailing-backslash.pristine" +printf '%s\n' 'unrelated=true' > "${bs_file}" +cp "${bs_file}" "${bs_pristine}" +if set_prop_encoded 'auth.token_secret' 'abc\' "${bs_file}" 2>/dev/null; then + echo "props.awk must refuse a value ending in an odd number of backslashes" >&2 + exit 1 +fi +cmp -s "${bs_file}" "${bs_pristine}" || { + echo "a refused set must leave the config byte-for-byte untouched" >&2 + exit 1 +} + +# An escaped backslash — two of them — is not a continuation, so it stays +# writable and replays byte for byte. Built from parts because a doubled +# backslash inside one literal is easy to write and hard to read back. +bs='\' +two_bs="abc${bs}${bs}" +set_prop_encoded 'auth.token_secret' "${two_bs}" "${bs_file}" +[[ "$(get_prop_encoded 'auth.token_secret' "${bs_file}")" == "${two_bs}" ]] +assert_line_count 1 '^unrelated=true$' "${bs_file}" From 5afbb4a3357d41798098ef9ec007d5dba6028b9c Mon Sep 17 00:00:00 2001 From: Adarsh Date: Wed, 23 Sep 2026 21:40:39 +0530 Subject: [PATCH 09/11] fix(docker): read properties and yaml the way the server does Addresses the blocking review. All eight findings reproduced first, and every fix below was reverted to confirm its own test goes red. props.awk, against java.util.Properties: - Line terminators. A bare CR ends a line in Java but not to getline, so a CR-only config reached the parser as one record: only its first key was ever seen, and rewriting that key replaced the whole record and dropped every later entry. Measured before: a file of three properties, one of them auth.authenticator; after set graph=..., one property left. Records are now split on \r\n, \n and \r, and the terminator each line arrived with is replayed so untouched lines keep their bytes. - Form feed. Java treats \f as whitespace either side of the separator, so `auth.authenticator=...` is that property; it was parsed into the key name instead, the guards read the file as unconfigured, and the append added a second competing definition. - Two read modes the guards needed: PROPS_MODE=has, which answers "is this key defined" without confusing an empty definition with no definition, and PROPS_DECODED=1 for callers that compare a value. Exit status 2 means an error and 1 means absent, so a caller wearing errexit cannot read an unreadable file as "not there" and append over it. gremlin-server.yaml: - yaml_auth_state moves to yamlscan.awk and answers about the mapping rather than the text. It reported named for `authentication: {} # authenticator: X`, and for a class nested under config:, both of which pass the parity check while Gremlin runs on AllowAllAuthenticator; and it reported nameless for a valid mapping with a comment line inside it, refusing a deployment that should start. Only a direct child counts, in block and flow form alike, comment text is not content, and an authenticator with no class is the nameless case. A separate file, which is also what keeps it free of the apostrophe that breaks a shell-quoted awk program. - check_auth_sides now runs on every start. Inside the PASSWORD branch only, a mounted REST-side authenticator with no yaml mapping was never validated. - enable-auth.sh wrapped the graph factory on a grep that matched a literal or backslash-escaped dot, so the legal `gremlin\u002egraph` spelling left the factory unwrapped with both servers told authentication was on. That read/write now goes through props.awk, and the embedded-CR workaround the grep needed goes with it. The script also stops on the first failed append: it exited 0 while a read-only mounted yaml left REST configured and the yaml not. props.awk moves to the assembly bin/ it is packaged from, which is where bin/enable-auth.sh finds it in the tarball as well as the image; the Dockerfile COPY of it is gone, since the image takes bin/ from the assembly. Verified: props.awk against java.util.Properties (javac/java 17) over a corpus of terminator, separator and escape forms, on read and on rewrite, 0 disagreements; yamlscan.awk over 21 shapes; both entrypoint suites; and the eight findings as a table, 8/8 failing at b8801a6 and 8/8 passing now. Not run here: mawk (no Linux container on this host), snakeyaml, and the CR-byte, chmod-mode and symlink assertions, which need a host where those primitives behave; they are gated to say so rather than pass quietly, and they run in CI. --- hugegraph-server/Dockerfile | 4 +- hugegraph-server/Dockerfile-hstore | 4 +- .../docker/docker-entrypoint-test.sh | 51 ++- .../docker/docker-entrypoint.sh | 95 ++-- .../docker/test/test-docker-entrypoint.sh | 425 +++++++++++++++++- .../hugegraph-dist/docker/yamlscan.awk | 257 +++++++++++ .../src/assembly/static/bin/enable-auth.sh | 131 ++++-- .../assembly/static/bin}/props.awk | 133 ++++-- 8 files changed, 969 insertions(+), 131 deletions(-) create mode 100644 hugegraph-server/hugegraph-dist/docker/yamlscan.awk rename hugegraph-server/hugegraph-dist/{docker => src/assembly/static/bin}/props.awk (69%) diff --git a/hugegraph-server/Dockerfile b/hugegraph-server/Dockerfile index f360adcb68..a2c059db5c 100644 --- a/hugegraph-server/Dockerfile +++ b/hugegraph-server/Dockerfile @@ -66,7 +66,9 @@ 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 . +# props.awk needs no COPY: it ships in the assembly bin/ above, which is also +# where bin/enable-auth.sh finds it. yamlscan.awk serves only the entrypoint. +COPY hugegraph-server/hugegraph-dist/docker/yamlscan.awk . RUN chmod 755 ./docker-entrypoint.sh EXPOSE 8080 diff --git a/hugegraph-server/Dockerfile-hstore b/hugegraph-server/Dockerfile-hstore index 81f1063d90..b7977b1724 100644 --- a/hugegraph-server/Dockerfile-hstore +++ b/hugegraph-server/Dockerfile-hstore @@ -68,7 +68,9 @@ 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 . +# props.awk needs no COPY: it ships in the assembly bin/ above, which is also +# where bin/enable-auth.sh finds it. yamlscan.awk serves only the entrypoint. +COPY hugegraph-server/hugegraph-dist/docker/yamlscan.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 6250ab4f14..47f11e55ae 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh @@ -23,7 +23,10 @@ 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" +# props.awk is packaged in the release bin/; the image gets it from there, and +# the entrypoint accepts it beside itself so this harness can stage either. +cp "${SCRIPT_DIR}/../src/assembly/static/bin/props.awk" "${TEST_HOME}/props.awk" +cp "${SCRIPT_DIR}/yamlscan.awk" "${TEST_HOME}/yamlscan.awk" touch "${TEST_HOME}/docker/init_complete" cat > "${TEST_HOME}/conf/rest-server.properties" <<'EOF' @@ -233,4 +236,50 @@ rm -f "${TEST_HOME}/docker/init_complete" ) grep -Fqx -- '-n' "${TEST_HOME}/docker/init-store-password" +# A mounted rest-server.properties that already carries auth.authenticator, +# with no matching yaml mapping and no PASSWORD given, used to start without a +# word: the parity check ran only inside the PASSWORD branch, so nothing ever +# compared the two sides and the server came up with REST enforcing and Gremlin +# on AllowAllAuthenticator. The check now runs on every start, and a refusal +# has to come before anything touches the backend. +printf '%s\n' 'host: 8182' > "${TEST_HOME}/conf/gremlin-server.yaml" +grep -qx 'auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' \ + "${TEST_HOME}/conf/rest-server.properties" || + printf '%s\n' \ + 'auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' \ + >> "${TEST_HOME}/conf/rest-server.properties" +rm -f "${TEST_HOME}/docker/init_complete" +before_calls="$(wc -l < "${TEST_HOME}/docker/init-store-calls")" +before_auth="$(wc -l < "${TEST_HOME}/docker/enable-auth-calls")" +status=0 +( + cd "${TEST_HOME}" + bash ./docker-entrypoint.sh +) || status=$? +if (( status == 0 )); then + echo "entrypoint must refuse a mounted REST-only authenticator with no PASSWORD" >&2 + exit 1 +fi +if [[ "$(wc -l < "${TEST_HOME}/docker/init-store-calls")" != "${before_calls}" ]]; then + echo "the refusal must happen before init-store runs" >&2 + exit 1 +fi +if [[ "$(wc -l < "${TEST_HOME}/docker/enable-auth-calls")" != "${before_auth}" ]]; then + echo "a refused start must not run enable-auth.sh" >&2 + exit 1 +fi + +# The same start is accepted once both sides agree, so the check above is a +# parity decision and not a blanket refusal to run without PASSWORD. +printf '%s\n' \ + 'authentication: {' \ + ' authenticator: org.apache.hugegraph.auth.StandardAuthenticator,' \ + ' config: {tokens: conf/rest-server.properties}' \ + '}' > "${TEST_HOME}/conf/gremlin-server.yaml" +rm -f "${TEST_HOME}/docker/init_complete" +( + cd "${TEST_HOME}" + bash ./docker-entrypoint.sh +) + echo "PASS: Docker entrypoint configures HStore discovery and authentication" diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh index 1f238152e9..5347b66c69 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh @@ -28,13 +28,40 @@ 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" +# separators, CR/CRLF/LF line terminators, 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 lives in the packaged bin/ directory because bin/enable-auth.sh +# reads properties with it too, and that assembly fileSet is what both the +# release tarball and this image are built from. Beside the entrypoint is only +# where the source tree and the tests put it. +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +props_from_env="${PROPS_AWK:-}" +yaml_from_env="${YAMLSCAN_AWK:-}" +PROPS_AWK="" +for candidate in "${props_from_env}" "${HERE}/props.awk" "${HERE}/bin/props.awk"; do + if [[ -n "${candidate}" && -f "${candidate}" ]]; then + PROPS_AWK="${candidate}" + break + fi +done +if [[ -z "${PROPS_AWK}" ]]; then + log "ERROR: props.awk not found beside the entrypoint or in bin/" + exit 1 +fi + +YAMLSCAN="" +for candidate in "${yaml_from_env}" "${HERE}/yamlscan.awk"; do + if [[ -n "${candidate}" && -f "${candidate}" ]]; then + YAMLSCAN="${candidate}" + break + fi +done +if [[ -z "${YAMLSCAN}" ]]; then + log "ERROR: yamlscan.awk not found beside the entrypoint" exit 1 fi @@ -80,43 +107,22 @@ get_prop_encoded() { } # What the top-level authentication mapping of gremlin-server.yaml says about -# authentication, as one of three states: +# authentication, as one of three states: none, named, nameless. # -# none no such mapping -# named the mapping carries an authenticator -# nameless the mapping exists but names no authenticator -# -# Only presence is asked for, never the class: the entrypoint does not copy a -# value between the two files any more, so quotes, inline comments and flow -# mappings stay snakeyaml's business instead of becoming a parser here. The -# key must start at column 0 — an `authentication:` nested under another -# mapping belongs to that feature, not to the Gremlin server, and reading it as -# the Gremlin one would let an unrelated class decide whether REST is -# authenticated while Gremlin stayed on TinkerPop's AllowAllAuthenticator. +# The question and its answer live in yamlscan.awk, which reads the mapping the +# way snakeyaml presents it to the server: only a column-0 `authentication` +# mapping counts, only its direct `authenticator` child names a class, comment +# text never counts as content, and a nested `config.authenticator` belongs to +# the config map rather than to the server. Those distinctions are the whole +# decision -- an earlier grep-shaped version of this function reported `named` +# for `authentication: {} # authenticator: X` and for a class nested under +# `config:`, which passed the REST/Gremlin parity check while Gremlin was +# running on AllowAllAuthenticator. yaml_auth_state() { local yaml="./conf/gremlin-server.yaml" [[ -f "${yaml}" ]] || { echo "none"; return 0; } - awk ' - /^authentication[ \t]*:/ { - inblk = 1 - have = 1 - # A flow mapping keeps the authenticator on the same line as the - # key, so it has to count there too; missing it would report a - # configured mapping as nameless and refuse a valid deployment. - if (match($0, /authenticator[ \t]*:/)) { named = 1; exit } - next - } - # Any other column-0 key ends the mapping. A blank or whitespace-only - # line does not, because YAML does not close a mapping on an empty line. - inblk && /^[^ \t]/ { inblk = 0 } - inblk && /^[ \t]+authenticator[ \t]*:/ { named = 1; exit } - END { - if (named) print "named" - else if (have) print "nameless" - else print "none" - } - ' "${yaml}" + awk -f "${YAMLSCAN}" "${yaml}" } # Authentication has to be configured on both sides or on neither. A mounted @@ -219,11 +225,16 @@ elif [[ -n "${AUTH_TOKEN_SECRET_ENCODED}" ]]; then set_prop_encoded "auth.token_secret" "${AUTH_TOKEN_SECRET_ENCODED}" \ "${GRAPH_CONF}" fi +# Both sides have to agree whether authentication is on, whatever the reason +# the container was started for. Running this only inside the PASSWORD branch +# below left a mounted rest-server.properties that carried auth.authenticator +# with no matching yaml mapping completely unvalidated: with no PASSWORD the +# entrypoint skipped the check, enable-auth.sh never ran, and the server came +# up with REST enforcing and Gremlin open. A refusal exits under set -e. +check_auth_sides + if [[ -n "${PASSWORD:-}" ]]; then set_prop "auth.admin_pa" "${PASSWORD}" "${REST_SERVER_CONF}" - # A refusal here exits the entrypoint under set -e, so enable-auth.sh can - # never run one-sided after it. - check_auth_sides # 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/test/test-docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh index 536facfc47..8c13198960 100644 --- a/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh @@ -36,8 +36,54 @@ for fn in encode_prop_value set_prop_encoded set_prop get_prop_encoded \ ' "${entrypoint}")" done log() { echo "[hugegraph-server-entrypoint] $*"; } -PROPS_AWK="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/props.awk" -export PROPS_AWK +static_bin="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../src/assembly/static/bin" && pwd)" +docker_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +PROPS_AWK="${static_bin}/props.awk" +YAMLSCAN="${docker_dir}/yamlscan.awk" +export PROPS_AWK YAMLSCAN + +# enable-auth.sh reads and writes .properties through props.awk, which the +# release assembly packages in the same bin/ directory. A test tree that runs +# the script therefore has to carry both, or it is not the layout it ships in. +install_enable_auth() { + local dir="$1" + mkdir -p "${dir}/bin" + cp "${static_bin}/enable-auth.sh" "${dir}/bin/enable-auth.sh" + cp "${static_bin}/props.awk" "${dir}/bin/props.awk" + chmod +x "${dir}/bin/enable-auth.sh" +} + +# ── What this host can actually be asked about ───────────────────────── +# CI runs these assertions on Ubuntu, where every one of them means what it +# says. Developed against a Windows host, three things silently stop being +# observations about props.awk and become observations about the platform: +# MSYS gawk opens text files in translation mode and drops the CR of a CRLF +# pair, chmod does not affect the mode stat reports, and a symlinked config is +# not a symlink. Each group is therefore gated on a probe of the host, and a +# skipped group says so out loud rather than passing quietly. +skip() { echo "note: skipped $1 -- this host cannot exercise it; it runs under CI" >&2; } + +probe="${test_dir}/probe" + +awk_sees_crlf_cr=0 +if [[ "$(printf 'x\r\n' | awk 'NR == 1 { print length($0) }')" == "2" ]]; then + awk_sees_crlf_cr=1 +fi +awk_sees_lone_cr=0 +if [[ "$(printf 'a\rb' | awk 'NR == 1 { print length($0) }')" == "3" ]]; then + awk_sees_lone_cr=1 +fi + +host_keeps_chmod=0 +printf '%s\n' x > "${probe}" +chmod 600 "${probe}" +[[ "$(stat -c '%a' "${probe}")" == "600" ]] && host_keeps_chmod=1 +rm -f "${probe}" + +host_keeps_symlink=0 +printf '%s\n' x > "${probe}-t" +ln -s "${probe}-t" "${probe}-l" 2>/dev/null && [[ -L "${probe}-l" ]] && host_keeps_symlink=1 +rm -f "${probe}-t" "${probe}-l" assert_replaced() { local separator="$1" @@ -279,9 +325,7 @@ mkdir -p "${sides_dir}/conf" # because check_auth_sides fails first under set -e. onesided_dir="${test_dir}/yaml-onesided" mkdir -p "${onesided_dir}/bin" "${onesided_dir}/conf/graphs" -cp "$(cd "$(dirname "${BASH_SOURCE[0]}")/../../src/assembly/static/bin" && pwd)/enable-auth.sh" \ - "${onesided_dir}/bin/enable-auth.sh" -chmod +x "${onesided_dir}/bin/enable-auth.sh" +install_enable_auth "${onesided_dir}" ( cd "${onesided_dir}" || exit 1 REST_SERVER_CONF="./conf/rest-server.properties" @@ -313,7 +357,12 @@ chmod +x "${onesided_dir}/bin/enable-auth.sh" # Untouched lines keep their CR bytes on rewrite. crlf_file="${test_dir}/config-crlf" printf 'auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator\r\n' > "${crlf_file}" -printf 'pd.peers=a,\\\r\n b\r\n' >> "${crlf_file}" +# The backslash goes through %s on purpose: in one format string, `\\\r` is +# reduced to a backslash followed by the letter r by some printf +# implementations, which quietly turns this continuation case into a plain line +# and makes the assertions below pass for the wrong reason. +printf '%s\r\n' 'pd.peers=a,\' >> "${crlf_file}" +printf ' b\r\n' >> "${crlf_file}" printf 'unrelated=true\r\n' >> "${crlf_file}" [[ "$(get_prop_encoded 'auth.authenticator' "${crlf_file}")" == \ "org.apache.hugegraph.auth.StandardAuthenticator" ]] @@ -321,9 +370,13 @@ printf 'unrelated=true\r\n' >> "${crlf_file}" set_prop 'auth.authenticator' 'com.example.NewAuth' "${crlf_file}" grep -q '^auth\.authenticator=com\.example\.NewAuth$' "${crlf_file}" [[ "$(get_prop_encoded 'pd.peers' "${crlf_file}")" == "a,b" ]] -if ! grep -q $'^unrelated=true\r$' "${crlf_file}"; then - echo "CRLF bytes of untouched lines must be preserved" >&2 - exit 1 +if (( awk_sees_crlf_cr )); then + if ! grep -q $'^unrelated=true\r$' "${crlf_file}"; then + echo "CRLF bytes of untouched lines must be preserved" >&2 + exit 1 + fi +else + skip "the CRLF byte check" fi # An escaped key is the same key: java.util.Properties unescapes the name, so @@ -356,19 +409,33 @@ mode_file="${test_dir}/config-mode" printf '%s\n' 'unrelated=true' > "${mode_file}" chmod 600 "${mode_file}" set_prop "init_store.enabled" "true" "${mode_file}" -[[ "$(stat -c '%a' "${mode_file}")" == "600" ]] grep -q '^init_store\.enabled=true$' "${mode_file}" grep -q '^unrelated=true$' "${mode_file}" [[ ! -e "${mode_file}.tmp" ]] [[ ! -e "${mode_file}.bak" ]] +if (( host_keeps_chmod )); then + [[ "$(stat -c '%a' "${mode_file}")" == "600" ]] +else + skip "the config-mode-preservation check" +fi target_file="${test_dir}/config-target" link_file="${test_dir}/config-link" -printf '%s\n' 'unrelated=true' > "${target_file}" -ln -s "${target_file}" "${link_file}" -set_prop "init_store.enabled" "true" "${link_file}" -[[ -L "${link_file}" ]] -grep -q '^init_store\.enabled=true$' "${target_file}" +if (( host_keeps_symlink )); then + # The whole block has to be gated, not just the -L check: where ln -s + # produces a copy instead, writing the link updates a regular file and the + # target stays untouched, which would fail for the host's reason. + printf '%s\n' 'unrelated=true' > "${target_file}" + ln -s "${target_file}" "${link_file}" + set_prop "init_store.enabled" "true" "${link_file}" + [[ -L "${link_file}" ]] || { + echo "a set must not replace a symlinked config with a regular file" >&2 + exit 1 + } + grep -q '^init_store\.enabled=true$' "${target_file}" +else + skip "the symlinked-config check" +fi # Two yaml shapes the scoping has to keep getting right: a sibling mapping # that carries its own authenticator must not hide the block's, and a commented @@ -412,10 +479,8 @@ class_dir="${test_dir}/authenticator-class" # it writes, so re-running it over one directory is not a clean case. run_enable_auth() { local dir="$1" want="$2" - mkdir -p "${dir}/bin" "${dir}/conf/graphs" - cp "$(cd "$(dirname "${BASH_SOURCE[0]}")/../../src/assembly/static/bin" && pwd)/enable-auth.sh" \ - "${dir}/bin/enable-auth.sh" - chmod +x "${dir}/bin/enable-auth.sh" + mkdir -p "${dir}/conf/graphs" + install_enable_auth "${dir}" printf '%s\n' 'gremlin.graph=org.apache.hugegraph.HugeFactory' \ > "${dir}/conf/graphs/hugegraph.properties" : > "${dir}/conf/rest-server.properties" @@ -453,9 +518,7 @@ class_dir="${test_dir}/authenticator-class" # auth mode, yet neither server was told to authenticate at all. empty_dir="${test_dir}/empty-config" mkdir -p "${empty_dir}/bin" "${empty_dir}/conf/graphs" -cp "$(cd "$(dirname "${BASH_SOURCE[0]}")/../../src/assembly/static/bin" && pwd)/enable-auth.sh" \ - "${empty_dir}/bin/enable-auth.sh" -chmod +x "${empty_dir}/bin/enable-auth.sh" +install_enable_auth "${empty_dir}" ( cd "${empty_dir}" || exit 1 : > conf/rest-server.properties @@ -594,3 +657,321 @@ two_bs="abc${bs}${bs}" set_prop_encoded 'auth.token_secret' "${two_bs}" "${bs_file}" [[ "$(get_prop_encoded 'auth.token_secret' "${bs_file}")" == "${two_bs}" ]] assert_line_count 1 '^unrelated=true$' "${bs_file}" + +# ── CR-only line terminators ────────────────────────────────────────── +# java.util.Properties ends a line at a bare CR as well, so a config written +# that way holds one property per CR-separated chunk. Reading it with a +# \n-only split made the entire file one record: only the first key was ever +# seen, and rewriting that key replaced the record with a single line, which +# silently deleted every property after it -- including auth.authenticator, so +# the file the server then read had no authentication configured at all. +if (( awk_sees_lone_cr )); then + cr_file="${test_dir}/config-cr" + printf 'graph=a\rpd.peers=b\rauth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator\r' \ + > "${cr_file}" + + [[ "$(get_prop_encoded 'graph' "${cr_file}")" == "a" ]] + [[ "$(get_prop_encoded 'pd.peers' "${cr_file}")" == "b" ]] + [[ "$(get_prop_encoded 'auth.authenticator' "${cr_file}")" == \ + "org.apache.hugegraph.auth.StandardAuthenticator" ]] + + cp "${cr_file}" "${cr_file}.before" + set_prop 'graph' 'org.apache.hugegraph.auth.HugeFactoryAuthProxy' "${cr_file}" + + # Every key that was there before is still there afterwards, with the + # values the rewrite was not about. + [[ "$(get_prop_encoded 'pd.peers' "${cr_file}")" == "b" ]] || { + echo "a CR-only config lost pd.peers when an unrelated key was rewritten" >&2 + exit 1 + } + [[ "$(get_prop_encoded 'auth.authenticator' "${cr_file}")" == \ + "org.apache.hugegraph.auth.StandardAuthenticator" ]] || { + echo "a CR-only config lost auth.authenticator when an unrelated key was rewritten" >&2 + exit 1 + } + [[ "$(get_prop_encoded 'graph' "${cr_file}")" == \ + "org.apache.hugegraph.auth.HugeFactoryAuthProxy" ]] + # One definition per key, so the rewrite replaced rather than appended. + # Counted on CR folded to LF because grep only ever starts a new line at + # LF, and a CR-only file is a single line to it. + count_records() { + local pattern="$1" file="$2" + # grep exits 1 on a zero count, which errexit would take as the + # interesting failure; the printed number is the answer here. + tr '\r' '\n' < "${file}" | grep -Ec "${pattern}" || true + } + [[ "$(count_records '^graph=' "${cr_file}")" == "1" ]] || { + echo "a CR-only rewrite must leave exactly one graph definition" >&2 + exit 1 + } + [[ "$(count_records '^auth\.authenticator=' "${cr_file}")" == "1" ]] || { + echo "a CR-only rewrite must leave exactly one auth.authenticator definition" >&2 + exit 1 + } + + # Mixed terminators in one file, the state an upgraded mounted volume + # actually reaches: CRLF from a Windows edit, CR from an old store(), LF + # from the image. + mix_file="${test_dir}/config-mixed-eol" + printf 'graph=a\rpd.peers=b\nauth.authenticator=c\r\nunrelated=d\n' > "${mix_file}" + [[ "$(get_prop_encoded 'graph' "${mix_file}")" == "a" ]] + [[ "$(get_prop_encoded 'pd.peers' "${mix_file}")" == "b" ]] + [[ "$(get_prop_encoded 'auth.authenticator' "${mix_file}")" == "c" ]] + [[ "$(get_prop_encoded 'unrelated' "${mix_file}")" == "d" ]] +fi + +# ── Form feed is separator whitespace to Java ───────────────────────── +# java.util.Properties counts \f as whitespace on both sides of the key/value +# boundary, so `auth.authenticator=...` is that property. Recognising only +# space and tab parsed the form feed into the key name instead, and a mounted +# config written that way read as unconfigured -- which the guards then answered +# by appending a second, competing definition. +ff_file="${test_dir}/config-formfeed" +printf 'auth.authenticator\fs=org.apache.hugegraph.auth.StandardAuthenticator\n' > "${ff_file}" +[[ "$(get_prop_encoded 'auth.authenticator' "${ff_file}")" == \ + "s=org.apache.hugegraph.auth.StandardAuthenticator" ]] || { + echo "a form feed before the separator must end the key, as it does in Java" >&2 + exit 1 +} +printf 'auth.authenticator\forg.apache.hugegraph.auth.X\n' > "${ff_file}" +[[ "$(get_prop_encoded 'auth.authenticator' "${ff_file}")" == \ + "org.apache.hugegraph.auth.X" ]] +printf 'auth.authenticator=\f1\n' > "${ff_file}" +[[ "$(get_prop_encoded 'auth.authenticator' "${ff_file}")" == "1" ]] +printf '\fauth.authenticator=1\n' > "${ff_file}" +[[ "$(get_prop_encoded 'auth.authenticator' "${ff_file}")" == "1" ]] +# A line that is only form feed whitespace is blank to Java, not a property. +printf '\f\f\ngraph=a\n' > "${ff_file}" +[[ "$(get_prop_encoded 'graph' "${ff_file}")" == "a" ]] +assert_line_count 1 '^graph=a$' "${ff_file}" + +# has-mode answers "is this key defined" without confusing an empty definition +# with no definition, which is what an append guard needs: appending a default +# on top of `auth.authenticator=` leaves the empty first definition in force. +has_file="${test_dir}/config-has" +printf 'auth.authenticator=\n' > "${has_file}" +if ! PROPS_MODE=has PROPS_KEY='auth.authenticator' PROPS_FILE="${has_file}" \ + awk -f "${PROPS_AWK}" /dev/null; then + echo "PROPS_MODE=has must report an empty definition as present" >&2 + exit 1 +fi +if PROPS_MODE=has PROPS_KEY='auth.graph_store' PROPS_FILE="${has_file}" \ + awk -f "${PROPS_AWK}" /dev/null; then + echo "PROPS_MODE=has must report an absent key as absent" >&2 + exit 1 +fi +# An unreadable file must not read as "absent": status 2 is what tells a caller +# wearing errexit to stop rather than append a default over a file it could not +# read. +status=0 +PROPS_MODE=has PROPS_KEY='k' PROPS_FILE="${test_dir}/no-such-file" \ + awk -f "${PROPS_AWK}" /dev/null 2>/dev/null || status=$? +if (( status != 2 )); then + echo "PROPS_MODE=has must exit 2 for an unreadable file, got ${status}" >&2 + exit 1 +fi + +# get with PROPS_DECODED=1 hands back the value as the server would see it, +# which is what a guard that compares a class name needs. +dec_file="${test_dir}/config-decoded" +printf 'gremlin\\u002egraph=org.apache.hugegraph.HugeFactory\n' > "${dec_file}" +[[ "$(PROPS_MODE=get PROPS_DECODED=1 PROPS_KEY='gremlin.graph' \ + PROPS_FILE="${dec_file}" awk -f "${PROPS_AWK}" /dev/null)" == \ + "org.apache.hugegraph.HugeFactory" ]] +[[ "$(PROPS_MODE=get PROPS_KEY='gremlin\u002egraph' PROPS_FILE="${dec_file}" \ + awk -f "${PROPS_AWK}" /dev/null)" == "" ]] + +# ── gremlin.graph spelled with a Unicode escape still gets wrapped ────── +# \u002e is a dot to java.util.Properties, so this is the plain HugeFactory and +# enable-auth.sh has to route authentication through it. The grep/sed pair +# matched only a literal or backslash-escaped dot, missed this one, and left the +# graph factory unwrapped while both servers had been told authentication was +# on -- the one remaining path where the REST side was configured and the graph +# behind it was not. +u2e_dir="${test_dir}/u2e-wrap" +mkdir -p "${u2e_dir}/conf/graphs" +install_enable_auth "${u2e_dir}" +: > "${u2e_dir}/conf/rest-server.properties" +: > "${u2e_dir}/conf/gremlin-server.yaml" +printf '%s\n' 'gremlin\u002egraph=org.apache.hugegraph.HugeFactory' \ + > "${u2e_dir}/conf/graphs/hugegraph.properties" +( + cd "${u2e_dir}" || exit 1 + unset AUTHENTICATOR_CLASS + ./bin/enable-auth.sh + if [[ "$(PROPS_MODE=get PROPS_DECODED=1 PROPS_KEY='gremlin.graph' \ + PROPS_FILE=./conf/graphs/hugegraph.properties \ + awk -f "${PROPS_AWK}" /dev/null)" != \ + "org.apache.hugegraph.auth.HugeFactoryAuthProxy" ]]; then + echo "a gremlin.graph key written as \\u002e must still be wrapped" >&2 + exit 1 + fi + # One definition, not the original left behind plus a new one. + if [[ "$(grep -c 'HugeFactory' ./conf/graphs/hugegraph.properties)" != "1" ]]; then + echo "wrapping a \\u002e-escaped key must not leave the old definition" >&2 + exit 1 + fi +) + +# A CR-only graph config wraps too, and keeps the keys around it. +if (( awk_sees_lone_cr )); then + crwrap_dir="${test_dir}/cr-wrap" + mkdir -p "${crwrap_dir}/conf/graphs" + install_enable_auth "${crwrap_dir}" + : > "${crwrap_dir}/conf/rest-server.properties" + : > "${crwrap_dir}/conf/gremlin-server.yaml" + printf 'gremlin.graph=org.apache.hugegraph.HugeFactory\rbackend=rocksdb\r' \ + > "${crwrap_dir}/conf/graphs/hugegraph.properties" + ( + cd "${crwrap_dir}" || exit 1 + unset AUTHENTICATOR_CLASS + ./bin/enable-auth.sh + [[ "$(get_prop_encoded 'backend' ./conf/graphs/hugegraph.properties)" == \ + "rocksdb" ]] || { + echo "wrapping a CR-only graph config dropped a later key" >&2 + exit 1 + } + [[ "$(get_prop_encoded 'gremlin.graph' ./conf/graphs/hugegraph.properties)" == \ + "org.apache.hugegraph.auth.HugeFactoryAuthProxy" ]] + ) +fi + +# ── A failed append must fail the script ────────────────────────────── +# The entrypoint runs enable-auth.sh and trusts its exit status, so a run that +# configures REST and then cannot write the yaml has to say so. Without +# errexit and per-write checks it exited 0 on exactly that half-done tree: the +# mounted read-only gremlin-server.yaml made the yaml append fail while both +# rest-server.properties appends succeeded. +ro_dir="${test_dir}/read-only-yaml" +mkdir -p "${ro_dir}/conf/graphs" +install_enable_auth "${ro_dir}" +: > "${ro_dir}/conf/rest-server.properties" +printf 'host: 8182\n' > "${ro_dir}/conf/gremlin-server.yaml" +printf '%s\n' 'gremlin.graph=org.apache.hugegraph.HugeFactory' \ + > "${ro_dir}/conf/graphs/hugegraph.properties" +( + cd "${ro_dir}" || exit 1 + unset AUTHENTICATOR_CLASS + chmod 444 conf/gremlin-server.yaml + status=0 + ./bin/enable-auth.sh 2>/dev/null || status=$? + chmod 644 conf/gremlin-server.yaml + if (( status == 0 )); then + echo "enable-auth.sh must exit nonzero when a config append fails" >&2 + exit 1 + fi + if grep -Eq '^[[:blank:]]*authentication[[:blank:]]*:' conf/gremlin-server.yaml; then + echo "the unwritable yaml file must not have been changed" >&2 + exit 1 + fi +) + +# ── yaml_auth_state answers about the mapping, not about the text ─────── +# Each case below is a mounted gremlin-server.yaml that a grep-shaped reader +# calls named while the Gremlin server runs without an authenticator. Reported +# parity on such a file is how REST ends up enforcing and Gremlin open, so the +# reader follows the mapping structure instead of the substring. +yaml_case() { + local want="$1" desc="$2" dir + shift 2 + dir="${test_dir}/yaml-$(printf '%s' "${desc}" | tr -c 'A-Za-z0-9' '-')" + mkdir -p "${dir}/conf" + printf '%s\n' "$@" > "${dir}/conf/gremlin-server.yaml" + ( + cd "${dir}" || exit 1 + got=$(yaml_auth_state) + if [[ "${got}" != "${want}" ]]; then + echo "yaml_auth_state: ${desc}: got ${got}, want ${want}" >&2 + exit 1 + fi + ) +} + +# A flow mapping that names nothing, with a commented-out authenticator behind +# it: the text is there, the key is not. +yaml_case nameless "flow empty with authenticator in a comment" \ + 'authentication: {} # authenticator: org.apache.hugegraph.auth.StandardAuthenticator' +# A comment line inside the mapping is not the end of it, so a valid +# deployment with a note between the keys must not be refused. +yaml_case named "column-zero comment inside the mapping" \ + 'authentication:' \ + '# configured by the operator' \ + ' authenticator: org.apache.hugegraph.auth.StandardAuthenticator' +# config is its own map, so an authenticator under it is the token store +# configuration and not the server authenticator. +yaml_case nameless "authenticator nested under config" \ + 'authentication:' \ + ' config:' \ + ' authenticator: org.apache.hugegraph.auth.StandardAuthenticator' +yaml_case nameless "authenticator nested inside a flow config" \ + 'authentication: {config: {authenticator: org.apache.hugegraph.auth.StandardAuthenticator}}' +# The positive cases a wrong reader must keep accepting. +yaml_case named "plain block child" \ + 'authentication:' \ + ' authenticator: org.apache.hugegraph.auth.StandardAuthenticator' +yaml_case named "direct flow child with siblings" \ + 'authentication: {config: {tokens: conf/rest-server.properties}, authenticator: org.apache.hugegraph.auth.StandardAuthenticator}' +yaml_case named "quoted key" \ + 'authentication:' \ + ' "authenticator": org.apache.hugegraph.auth.StandardAuthenticator' +# An authenticator key that names no class leaves the server on +# AllowAllAuthenticator, so it is the nameless case. +yaml_case nameless "direct authenticator with no value" \ + 'authentication:' \ + ' authenticator:' +yaml_case nameless "direct authenticator set to null" \ + 'authentication:' \ + ' authenticator: null' +# An `authentication:` belonging to another mapping is not the server's. +yaml_case none "authentication nested under another key" \ + 'server:' \ + ' authentication:' \ + ' authenticator: org.apache.hugegraph.auth.StandardAuthenticator' +yaml_case none "no authentication anywhere" \ + 'host: 8182' \ + 'port: 1' +# A sibling key at column zero closes the mapping; an authenticator after it +# belongs to the sibling, not to authentication. +yaml_case nameless "sibling key closes the mapping" \ + 'authentication:' \ + ' handler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler' \ + 'metrics:' \ + ' authenticator: org.apache.hugegraph.auth.StandardAuthenticator' + +# ── Mounted one-sided config is refused with no PASSWORD ─────────────── +# check_auth_sides used to run only inside the PASSWORD branch, so a mounted +# rest-server.properties that already carried auth.authenticator and a yaml +# without a matching mapping was never validated at all: the entrypoint skipped +# the check, never called enable-auth.sh, and started the server with REST +# enforcing and Gremlin open. The parity check now runs on every start. +mounted_dir="${test_dir}/mounted-one-sided" +mkdir -p "${mounted_dir}/conf/graphs" +( + cd "${mounted_dir}" || exit 1 + # check_auth_sides reads these two paths, which the entrypoint sets at the + # top of a run; this block calls the guard directly, as the other unit + # groups here do. + REST_SERVER_CONF="./conf/rest-server.properties" + GRAPH_CONF="./conf/graphs/hugegraph.properties" + printf '%s\n' \ + 'restserver.url=http://127.0.0.1:8080' \ + 'auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' \ + > conf/rest-server.properties + printf '%s\n' 'host: 8182' > conf/gremlin-server.yaml + printf '%s\n' 'backend=rocksdb' > conf/graphs/hugegraph.properties + if check_auth_sides; then + echo "check_auth_sides must refuse REST configured with yaml not" >&2 + exit 1 + fi + # And it accepts the two balanced states, so this is not just a refusal: + printf '%s\n' \ + 'authentication:' \ + ' authenticator: org.apache.hugegraph.auth.StandardAuthenticator' \ + > conf/gremlin-server.yaml + check_auth_sides + printf '%s\n' 'host: 8182' > conf/gremlin-server.yaml + printf '%s\n' \ + 'restserver.url=http://127.0.0.1:8080' \ + > conf/rest-server.properties + check_auth_sides +) diff --git a/hugegraph-server/hugegraph-dist/docker/yamlscan.awk b/hugegraph-server/hugegraph-dist/docker/yamlscan.awk new file mode 100644 index 0000000000..4c127eb7c5 --- /dev/null +++ b/hugegraph-server/hugegraph-dist/docker/yamlscan.awk @@ -0,0 +1,257 @@ +# 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. +# +# yamlscan.awk -- does the top-level `authentication` mapping of a Gremlin +# server YAML file name an authenticator? Prints exactly one of: +# +# none there is no top-level authentication mapping +# nameless the mapping exists but names no authenticator class +# named the mapping names an authenticator class +# +# The entrypoint asks this one question to decide whether +# rest-server.properties and gremlin-server.yaml configure authentication +# together. Getting it wrong toward "named" is how REST ends up enforcing +# StandardAuthenticator while Gremlin silently falls back to TinkerPop +# AllowAllAuthenticator, so the answer has to follow the same structure +# snakeyaml hands to the server, within the subset of YAML that shipped and +# mounted configs use: +# +# 1. the mapping must start at column 0 -- an `authentication:` nested under +# some other key belongs to that feature, not to the Gremlin server; +# 2. only a direct child `authenticator` counts -- a class reached through +# `authentication.config`, or through any other nested mapping, is not +# the server authenticator, because TinkerPop keeps `config` as its own +# map; +# 3. `#` outside quotes starts a comment: text behind one is not content, +# and a comment-only line is neither a child nor the end of the mapping; +# 4. in a flow mapping the key must sit at depth one between the braces, so +# `{authenticator: X}` names a class while `{config: {authenticator: X}}` +# does not; +# 5. a direct `authenticator` whose value is empty, `null` or `~` names no +# class -- the server reads the key, gets nothing and leaves +# authentication off, which is the nameless case that must be refused. +# +# Quote characters come from sprintf so this file holds no literal apostrophe: +# an awk program written into a single-quoted shell string breaks on one, and +# that has cost this repo twice already. + +function apos() { return sprintf("%c", 39) } +function dquo() { return sprintf("%c", 34) } + +function ltrim(s) { sub(/^[ \t]+/, "", s); return s } +function rtrim(s) { sub(/[ \t]+$/, "", s); return s } +function trim(s) { return rtrim(ltrim(s)) } + +function is_quote(c) { return c == apos() || c == dquo() } + +# Remove an unquoted trailing comment together with the whitespace that has to +# precede the `#` for it to be a comment rather than part of a scalar. +function strip_comment(s, i, n, c, q, prev) { + q = "" + prev = "" + n = length(s) + for (i = 1; i <= n; i++) { + c = substr(s, i, 1) + if (q != "") { + if (c == q) q = "" + } else if (is_quote(c)) { + q = c + } else if (c == "#" && (prev == "" || prev == " " || prev == "\t")) { + return rtrim(substr(s, 1, i - 1)) + } + prev = c + } + return s +} + +# How many whitespace characters open the line, i.e. its block nesting level. +function indent_of(s, i, n, c) { + n = length(s) + i = 1 + while (i <= n) { + c = substr(s, i, 1) + if (c != " " && c != "\t") break + i++ + } + return i - 1 +} + +# One layer of matching quotes off a key or scalar. +function unquote(s, f) { + s = trim(s) + if (length(s) >= 2) { + f = substr(s, 1, 1) + if ((f == apos() || f == dquo()) && substr(s, length(s), 1) == f) + return substr(s, 2, length(s) - 2) + } + return s +} + +# Split `name: value` at the first colon outside quotes that is followed by end +# of line or a space, which is what makes a colon inside `http://host` part of +# the scalar. Results go to K_TXT / V_TXT because awk returns one value. +function split_pair(s, i, n, c, q) { + q = "" + n = length(s) + for (i = 1; i <= n; i++) { + c = substr(s, i, 1) + if (q != "") { + if (c == q) q = "" + continue + } + if (is_quote(c)) { q = c; continue } + if (c != ":") continue + if (i == n || substr(s, i + 1, 1) ~ /^[ \t]/) { + K_TXT = rtrim(substr(s, 1, i - 1)) + V_TXT = ltrim(substr(s, i + 1)) + return 1 + } + } + return 0 +} + +function names_authenticator(k) { return unquote(k) == "authenticator" } + +# An authenticator entry only counts when it actually names a class. +function names_class(v) { + v = trim(v) + return v != "" && v != "null" && v != "~" +} + +# Report and stop. Output happens in END only, because awk runs END after +# `exit` and a second print there would emit two states on one run. +function finish(r) { RESULT = r; exit } + +# Feed one line of a flow collection to the brace scanner. DEPTH counts open +# collections; keys and values are only read at depth one, which is what makes +# a nested mapping under `config` invisible to it. FSET records a direct +# authenticator that names a class. Returns 1 once the outermost collection +# has closed. +function scan_flow(s, i, n, c, q) { + n = length(s) + q = "" + for (i = 1; i <= n; i++) { + c = substr(s, i, 1) + if (q != "") { + if (FST == "key") CUR = CUR c + if (c == q) q = "" + continue + } + if (is_quote(c)) { + q = c + if (FST == "key") CUR = CUR c + continue + } + if (c == "{" || c == "[") { + DEPTH++ + CUR = "" + # Past depth one the whole entry is nested content and is skipped, + # including an authenticator key inside it. + FST = (DEPTH == 1 ? "key" : "skip") + continue + } + if (c == "}" || c == "]") { + if (DEPTH == 1 && FST == "val") commit_val() + DEPTH-- + CUR = "" + if (DEPTH == 0) { FST = "key"; return 1 } + FST = "skip" + continue + } + if (DEPTH != 1) continue + if (c == ":") { + if (FST == "key") { + CUR_KEY = CUR + CUR_VAL = "" + FST = "val" + } + CUR = "" + continue + } + if (c == ",") { + if (FST == "val") commit_val() + FST = "key" + CUR = "" + continue + } + if (c == " " || c == "\t") { + # A space ends an unquoted key but never carries a value byte. + continue + } + if (FST == "key") CUR = CUR c + else if (FST == "val") CUR_VAL = CUR_VAL c + } + return 0 +} + +# Close out the depth-one entry that was being read when a `,` or `}` arrived. +function commit_val( k) { + k = CUR_KEY + if (names_authenticator(k) && names_class(CUR_VAL)) FSET = 1 +} + +BEGIN { + DEPTH = 0 + FST = "key" + CUR = "" + CUR_KEY = "" + CUR_VAL = "" + FSET = 0 + found = 0 + child = -1 + flow = 0 + RESULT = "" +} + +{ + line = strip_comment($0) + + if (!found) { + if (line ~ /^[ \t]/) next + if (!split_pair(line)) next + if (unquote(K_TXT) != "authentication") next + found = 1 + if (substr(V_TXT, 1, 1) == "{") { + flow = 1 + if (scan_flow(V_TXT)) finish(FSET ? "named" : "nameless") + next + } + # Anything else on the key line -- a scalar, a sequence, nothing -- is + # not a mapping that names a class. Reading `authentication: some.Name` + # as named would accept a config the server cannot use. + next + } + + if (flow) { + if (scan_flow(line)) finish(FSET ? "named" : "nameless") + next + } + + if (trim(line) == "") next + # A column-0 line after the comment was stripped is a sibling key, so the + # mapping has ended. + if (indent_of(line) == 0) finish("nameless") + + if (!split_pair(line)) next + if (child < 0) child = indent_of(line) + if (indent_of(line) != child) next + if (names_authenticator(K_TXT) && names_class(V_TXT)) finish("named") +} + +END { + if (RESULT != "") { print RESULT; exit } + if (!found) print "none" + else print "nameless" +} 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 8737d20088..639003f702 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 @@ -16,6 +16,8 @@ # limitations under the License. # +set -euo pipefail + function abs_path() { SOURCE="${BASH_SOURCE[0]}" while [[ -h "$SOURCE" ]]; do @@ -34,42 +36,102 @@ GREMLIN_SERVER_CONF="gremlin-server.yaml" REST_SERVER_CONF="rest-server.properties" GRAPH_CONF="hugegraph.properties" +fail() { + echo "enable-auth.sh: $*" >&2 + exit 1 +} + +# Reading and writing .properties files goes through props.awk, the same helper +# the docker entrypoint uses, because the keys below can be spelled in every way +# java.util.Properties accepts: `=`/`:`/bare-whitespace separators, a form feed +# as whitespace, `\.` or `\u002e` for the dots, and LF, CRLF or CR line +# terminators. grep and sed see a different file. A legal +# `gremlin\u002egraph=org.apache.hugegraph.HugeFactory` matched no pattern at +# all, so the factory was never wrapped for auth even though both servers were +# told authentication was on -- and the CR byte that the previous pattern had +# to be handed a carriage return for is now handled by the reader itself. +# +# props.awk is packaged in this same bin/ directory by the release assembly, so +# it is present in the tarball and in the image; the entrypoint also exports +# PROPS_AWK when it calls this script. +for candidate in "${PROPS_AWK:-}" "${BIN}/props.awk" "${TOP}/props.awk"; do + if [[ -n "${candidate}" && -f "${candidate}" ]]; then + PROPS_AWK="${candidate}" + break + fi +done +[[ -n "${PROPS_AWK:-}" ]] || fail "props.awk not found beside this script" + +# Exit status of the reader is meaningful: 1 means the key has no definition, +# 2 means props.awk could not do its job. Only 1 is an acceptable answer here. +props_has() { + local status=0 + PROPS_MODE=has PROPS_KEY="$1" PROPS_FILE="$2" awk -f "${PROPS_AWK}" /dev/null || status=$? + if (( status > 1 )); then + fail "cannot read $2" + fi + return "${status}" +} + +props_get() { + local status=0 value + value=$(PROPS_MODE=get PROPS_DECODED=1 PROPS_KEY="$1" PROPS_FILE="$2" \ + awk -f "${PROPS_AWK}" /dev/null) || status=$? + if (( status > 0 )); then + fail "cannot read $2" + fi + printf '%s' "${value}" +} + +props_set() { + # The only values written here are Java class names, whose characters need + # no properties escaping; anything else would have to go through the + # entrypoint's encoder first. + case "$2" in + *[!A-Za-z0-9_\.\$]*) fail "refusing to write an unescaped value: $2" ;; + esac + PROPS_MODE=set PROPS_KEY="$1" PROPS_VALUE_ENCODED="$2" PROPS_FILE="$3" \ + awk -f "${PROPS_AWK}" /dev/null || fail "cannot update $3" +} + # make a backup BAK_CONF="$TOP/conf-bak" if [ ! -d "$BAK_CONF" ]; then - mkdir -p "$BAK_CONF" - 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" + mkdir -p "$BAK_CONF" || fail "cannot create ${BAK_CONF}" + cp "${CONF}/${GREMLIN_SERVER_CONF}" "${BAK_CONF}/${GREMLIN_SERVER_CONF}.bak" || + fail "cannot back up ${GREMLIN_SERVER_CONF}" + cp "${CONF}/${REST_SERVER_CONF}" "${BAK_CONF}/${REST_SERVER_CONF}.bak" || + fail "cannot back up ${REST_SERVER_CONF}" + cp "${CONF}/graphs/${GRAPH_CONF}" "${BAK_CONF}/${GRAPH_CONF}.bak" || + fail "cannot back up ${GRAPH_CONF}" 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). -# 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. - -# Appended with `>>` rather than `sed -i '$a\...'`: GNU sed's `$` address -# never matches when the file has no lines, so on an empty mounted config -# every append below silently did nothing. Neither the REST -# `auth.authenticator` nor the yaml `authentication:` block was written, -# while the entrypoint had already applied PASSWORD and init-store had run -# in auth mode — the servers then came up unauthenticated with no error. -# `sed -i '$a'` also closed the previous last line for us, which `>>` does -# not, so a file without a trailing newline gets one first. +# The appends below are guarded per file and skip any file that already carries +# the property, so they are no-ops on a mounted config or a re-run. 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. +# +# Appended with `>>` rather than `sed -i '$a\...'`: GNU sed's `$` address never +# matches when the file has no lines, so on an empty mounted config every append +# silently did nothing. `sed -i '$a'` also closed the previous last line for us, +# which `>>` does not, so a file without a trailing newline gets one first. +# +# Every write here has to be seen to succeed. The docker entrypoint runs this +# script and trusts its exit status, and a partially updated tree -- REST +# configured, yaml append refused by a read-only mounted file -- is exactly the +# one-sided state the entrypoint refuses to start with. Without errexit and +# these checks the script exited 0 on that half-done job. append_lines() { local file="$1" shift + if [[ ! -w "${file}" ]]; then + fail "cannot append to ${file}: not writable" + fi if [[ -s "${file}" && -n "$(tail -c 1 "${file}")" ]]; then - printf '\n' >> "${file}" + printf '\n' >> "${file}" || fail "cannot append to ${file}" fi - printf '%s\n' "$@" >> "${file}" + printf '%s\n' "$@" >> "${file}" || fail "cannot append to ${file}" } AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}" @@ -83,19 +145,18 @@ if ! grep -Eq '^[[:blank:]]*authentication[[:blank:]]*:' "${CONF}/${GREMLIN_SERV '}' fi -if ! grep -Eq '^[[:blank:]]*auth[\\]?\.authenticator[[:blank:]]*([:=]|[[:blank:]])' "${CONF}/${REST_SERVER_CONF}"; then +if ! props_has "auth.authenticator" "${CONF}/${REST_SERVER_CONF}"; then append_lines "${CONF}/${REST_SERVER_CONF}" "auth.authenticator=${AUTHENTICATOR_CLASS}" fi -if ! grep -Eq '^[[:blank:]]*auth[\\]?\.graph_store[[:blank:]]*([:=]|[[:blank:]])' "${CONF}/${REST_SERVER_CONF}"; then +if ! props_has "auth.graph_store" "${CONF}/${REST_SERVER_CONF}"; then append_lines "${CONF}/${REST_SERVER_CONF}" 'auth.graph_store=hugegraph' fi -# 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 "^[[:blank:]]*gremlin[\\\\]?\\.graph[[:blank:]]*([:=]|[[:blank:]])[[:blank:]]*org\\.apache\\.hugegraph\\.HugeFactory[[:blank:]]*${CR}?$" "${CONF}/graphs/${GRAPH_CONF}"; then - sed -i -E "s#^([[:blank:]]*gremlin[\\\\]?\\.graph[[:blank:]]*([:=]|[[:blank:]])[[:blank:]]*)org\\.apache\\.hugegraph\\.HugeFactory#\\1org.apache.hugegraph.auth.HugeFactoryAuthProxy#" "${CONF}/graphs/${GRAPH_CONF}" +# Wrap the graph factory only when it really is the plain HugeFactory, which is +# a question about the decoded value, so it goes through the same reader. +GRAPH_FACTORY=$(props_get "gremlin.graph" "${CONF}/graphs/${GRAPH_CONF}") +if [[ "${GRAPH_FACTORY}" == "org.apache.hugegraph.HugeFactory" ]]; then + props_set "gremlin.graph" "org.apache.hugegraph.auth.HugeFactoryAuthProxy" \ + "${CONF}/graphs/${GRAPH_CONF}" fi diff --git a/hugegraph-server/hugegraph-dist/docker/props.awk b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/props.awk similarity index 69% rename from hugegraph-server/hugegraph-dist/docker/props.awk rename to hugegraph-server/hugegraph-dist/src/assembly/static/bin/props.awk index 1dfb7ab8ae..244202b821 100644 --- a/hugegraph-server/hugegraph-dist/docker/props.awk +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/props.awk @@ -23,7 +23,12 @@ # 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 +# print the value of K's first logical definition, in the on-disk +# escaped form; with PROPS_DECODED=1 print it as java.util.Properties +# would hand it to the server. Always exits 0. +# PROPS_MODE=has PROPS_KEY=K PROPS_FILE=F +# print nothing; exit 0 when K has any definition at all, empty +# included, 1 when it has none, 2 on an error # 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 @@ -34,9 +39,11 @@ # # Grammar implemented (java.util.Properties line reader + the # first-definition-wins rule Configuration.getString applies): +# - physical lines end at \r\n, \n or a bare \r, as in java.util.Properties # - '#' / '!' comments and blank lines -# - '=' / ':' / whitespace separators, with whitespace then an optional -# single '=' or ':' accepted as one separator +# - '=' / ':' / whitespace separators, where the whitespace Java counts is +# space, tab and form feed, 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 @@ -48,7 +55,10 @@ function die(msg) { printf "props.awk: %s\n", msg > "/dev/stderr" - exit 1 + # 2 for an error, so a caller that reads exit status 1 as "the key is not + # there" (PROPS_MODE=has) cannot mistake an unreadable file for an absent + # property and append a definition on top of one it failed to read. + exit 2 } function hex_digit(c) { @@ -101,11 +111,14 @@ function trailing_backslashes(s, n, k) { } function is_skipped(raw) { - return raw ~ /^[ \t]*([#!]|$)/ + return raw ~ /^[ \t\f]*([#!]|$)/ } # 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. +# Java treats form feed as whitespace on both sides of the separator, so +# `auth.authenticator=...` is one property here too; reading it as part of +# the key name made a valid mounted configuration invisible to the guards. function split_kv(s, n, i, c, esc, sep_at, rest) { n = length(s) esc = 0 @@ -114,7 +127,7 @@ function split_kv(s, n, i, c, esc, sep_at, rest) { 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 (c == "=" || c == ":" || c == " " || c == "\t" || c == "\f") { sep_at = i; break } } if (sep_at == 0) { K_RAW = s @@ -127,11 +140,11 @@ function split_kv(s, n, i, c, esc, sep_at, rest) { if (c == "=" || c == ":") { rest = substr(rest, 2) } else { - sub(/^[ \t]+/, "", rest) + sub(/^[ \t\f]+/, "", rest) c = substr(rest, 1, 1) if (c == "=" || c == ":") rest = substr(rest, 2) } - sub(/^[ \t]+/, "", rest) + sub(/^[ \t\f]+/, "", rest) V_RAW = rest } @@ -140,26 +153,63 @@ function shquote(s) { return "'" s "'" } +# java.util.Properties ends a physical line at \r\n, \n or a bare \r, but +# getline splits on \n alone. A properties file saved with CR-only endings -- +# which java.util.Properties writes for a lone `store()` on some platforms, and +# which a mounted config can arrive with -- therefore reached the parser as one +# enormous record: only its first key was ever seen, and rewriting that key +# replaced the whole record and dropped every later entry, including +# auth.authenticator. So the file is re-scanned for terminators here. +# +# RAW[] keeps the exact bytes of each line and RAWTERM[] its terminator, so a +# rewrite still replays untouched lines byte-for-byte. A file whose last line +# carries no terminator gets a \n, which is what the replay did before. +function scan_records(s, i, n, c, start, term, len, cnt) { + n = length(s) + cnt = 0 + start = 1 + i = 1 + while (i <= n) { + c = substr(s, i, 1) + if (c != "\r" && c != "\n") { i++; continue } + if (c == "\r" && substr(s, i + 1, 1) == "\n") { + term = "\r\n" + len = 2 + } else { + term = c + len = 1 + } + cnt++ + RAW[cnt] = substr(s, start, i - start) + RAWTERM[cnt] = term + start = i + len + i = start + } + if (start <= n) { + cnt++ + RAW[cnt] = substr(s, start) + RAWTERM[cnt] = "" + } + return cnt +} + # 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, rc, nl, stripped, next_raw, start, logical) { - NLINES = 0 - while ((rc = (getline raw < file)) > 0) { - NLINES++ - RAW[NLINES] = raw - } +function props_load(file, raw, rc, content, nl, stripped, next_raw, start, logical) { + content = "" + while ((rc = (getline raw < file)) > 0) + content = content raw "\n" if (rc == -1) die("cannot read " file) close(file) + NLINES = scan_records(content) + NBLOCK = 0 for (nl = 1; nl <= NLINES; nl++) { - raw = RAW[nl] - # CRLF: java.util.Properties drops the line terminator, so one - # trailing CR is stripped for parsing only. RAW[] keeps the byte - # so props_set replays untouched lines byte-for-byte. - stripped = raw - sub(/\r$/, "", stripped) + # RAW[] holds one java.util.Properties physical line with its terminator + # already removed, so no CR stripping is needed here. + stripped = RAW[nl] if (is_skipped(stripped)) { NBLOCK++ BTYPE[NBLOCK] = "skip" @@ -173,15 +223,14 @@ function props_load(file, raw, rc, nl, stripped, next_raw, start, logical) { logical = substr(logical, 1, length(logical) - 1) nl++ next_raw = RAW[nl] - sub(/\r$/, "", next_raw) - sub(/^[ \t]+/, "", next_raw) + sub(/^[ \t\f]+/, "", 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) + sub(/^[ \t\f]+/, "", logical) split_kv(logical) NBLOCK++ BTYPE[NBLOCK] = "entry" @@ -231,8 +280,14 @@ function props_set(file, key, enc_val, tmp, bak, cmd, b, first, ln, msg, nbs) if (b == first) { printf "%s=%s\n", key, enc_val > tmp } else { - for (ln = BFIRST[b]; ln <= BLAST[b]; ln++) - print RAW[ln] > tmp + for (ln = BFIRST[b]; ln <= BLAST[b]; ln++) { + # Replay the line with the terminator it was read with, so a + # CRLF or CR-only config keeps its endings on lines the + # rewrite does not touch. + msg = RAWTERM[ln] + if (msg == "") msg = "\n" + printf "%s%s", RAW[ln], msg > tmp + } } } if (first == 0) @@ -273,27 +328,47 @@ function props_set(file, key, enc_val, tmp, bak, cmd, b, first, ln, msg, nbs) die("cannot remove " tmp " and " bak " after the copy-back") } -function props_get(file, key, b) { +function props_get(file, key, decoded, b) { props_load(file) for (b = 1; b <= NBLOCK; b++) { if (BTYPE[b] == "entry" && BKEY[b] == key) { - print BVAL[b] + if (decoded) print unescape(BVAL[b]) + else print BVAL[b] return } } + # Absence prints nothing and is NOT an exit status: callers assign from + # command substitution (`rest=$(get_prop ...)`) under a shell with errexit + # on, where a nonzero status would abort the entrypoint over a merely + # missing property. PROPS_MODE=has is the mode that reports by status. +} + +# Exit status only: 0 when the key has any definition at all, including an +# empty one. Guards that append a default must not treat `auth.authenticator=` +# as absent, because appending a second definition leaves the empty first one +# in force under first-definition-wins. +function props_has(file, key, b) { + props_load(file) + for (b = 1; b <= NBLOCK; b++) { + if (BTYPE[b] == "entry" && BKEY[b] == key) return 0 + } + return 1 } BEGIN { mode = ENVIRON["PROPS_MODE"] key = ENVIRON["PROPS_KEY"] file = ENVIRON["PROPS_FILE"] + decoded = (ENVIRON["PROPS_DECODED"] == "1") if (file == "" || key == "") die("PROPS_FILE and PROPS_KEY must be set") if (mode == "get") { - props_get(file, key) + props_get(file, key, decoded) + } else if (mode == "has") { + if (props_has(file, key)) exit 1 } else if (mode == "set") { props_set(file, key, ENVIRON["PROPS_VALUE_ENCODED"]) } else { - die("PROPS_MODE must be get or set") + die("PROPS_MODE must be get, has or set") } } From 2c9ebaf0bcbbad44e1008b312390e0a7a74e0ba4 Mon Sep 17 00:00:00 2001 From: Adarsh Date: Wed, 23 Sep 2026 23:00:18 +0530 Subject: [PATCH 10/11] fix(docker): let enable-auth.sh answer what the entrypoint asks check_auth_sides and enable-auth.sh decide one question from two files, and the guards here asked it about presence while the entrypoint asks it about the value. Both findings were reproduced against this script before anything changed, and both new test groups were run against the unpatched script to confirm they go red. - Column-0 `authentication` only. The yaml guard accepted the key at any indentation while yamlscan.awk counts only a column-0 mapping, so a config that nests `authentication` under another feature read as `none`: parity held, the entrypoint ran this script, and the guard then saw the nested key, skipped the append and wrote the REST side alone. Gremlin stayed on TinkerPop's AllowAllAuthenticator under a StandardAuthenticator REST. The nested block is still left byte-identical, and that is asserted too. - A defined-but-empty authenticator is the unconfigured side. props_has reports `auth.authenticator=`, a bare `auth.authenticator` line and `auth.authenticator= ` as present, and they are: measured against java.util.Properties all three parse to the empty string, and HugeAuthenticator.loadAuthenticator returns null for that, so REST serves without authentication. The presence guard skipped the append on exactly the side that needed it. The two REST defaults now go through props_set, which rewrites an existing definition where it stands -- the placeholder is replaced rather than followed by a second definition that first-definition-wins would bury -- and appends when there is nothing to replace. An operator-written class is still never overwritten. - props_has had no caller left once both defaults moved, so it is gone. PROPS_MODE=has stays in props.awk: it is a documented answer to a different question and is still covered directly. Not verified here: CR-only and CRLF gremlin-server.yaml, which MSYS text mode cannot observe, and mawk, which is not installed on this host. Both are exercised by CI on Ubuntu. --- .../docker/test/test-docker-entrypoint.sh | 131 ++++++++++++++++++ .../src/assembly/static/bin/enable-auth.sh | 63 +++++---- 2 files changed, 170 insertions(+), 24 deletions(-) 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 8c13198960..411c711a6e 100644 --- a/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh @@ -975,3 +975,134 @@ mkdir -p "${mounted_dir}/conf/graphs" > conf/rest-server.properties check_auth_sides ) + +# ── Trees the entrypoint hands to enable-auth.sh ─────────────────────── +# check_auth_sides and enable-auth.sh answer one question from two files, so +# they have to answer it the same way. Every case below is a tree that +# check_auth_sides ACCEPTS -- which is why the entrypoint goes on to run +# enable-auth.sh -- and where the old guard here wrote only one side: it asked +# whether a key or a block was present, while the entrypoint asks whether a +# value names a class. Those disagree for an `authentication:` nested under +# another feature and for a defined-but-empty `auth.authenticator`, and the +# result was REST enforcing StandardAuthenticator beside a Gremlin left on +# TinkerPop's AllowAllAuthenticator. +parity_dir="${test_dir}/enable-auth-parity" + +# bootstrap : the layout the script ships in, with props.awk beside it. +bootstrap_tree() { + local dir="$1" + rm -rf "${dir}" + mkdir -p "${dir}/conf/graphs" + install_enable_auth "${dir}" + printf '%s\n' 'gremlin.graph=org.apache.hugegraph.HugeFactory' \ + > "${dir}/conf/graphs/hugegraph.properties" + : > "${dir}/conf/gremlin-server.yaml" + : > "${dir}/conf/rest-server.properties" +} + +# The class the server would read, through the same reader rather than through +# grep: an appended second definition looks correct to grep and is invisible +# here, which is the failure these cases are about. +rest_class() { + PROPS_MODE=get PROPS_DECODED=1 PROPS_KEY=auth.authenticator \ + PROPS_FILE="$1/conf/rest-server.properties" awk -f "${PROPS_AWK}" /dev/null +} + +gremlin_state() { + ( cd "$1" && yaml_auth_state ) +} + +# accepted_then_both_sides -- refuse to test a tree the +# entrypoint would never run the script on, then require both sides named. +# REST_SERVER_CONF is a top-level assignment in docker-entrypoint.sh and this +# group evals only the functions, so each call has to carry it: unset, the REST +# side reads as unconfigured whatever the file says, and a one-sided tree would +# be waved through the very guard being asserted. +sides_agree() { + ( cd "$1" && REST_SERVER_CONF="./conf/rest-server.properties" check_auth_sides ) +} + +accepted_then_both_sides() { + local desc="$1" dir="$2" state class + if ! sides_agree "${dir}" >/dev/null 2>&1; then + echo "${desc}: check_auth_sides refused this tree, so enable-auth.sh + is never reached -- the case no longer tests what it was written for" >&2 + exit 1 + fi + ( cd "${dir}" && unset AUTHENTICATOR_CLASS && ./bin/enable-auth.sh ) || { + echo "${desc}: enable-auth.sh failed" >&2 + exit 1 + } + state=$(gremlin_state "${dir}") + if [[ "${state}" != "named" ]]; then + echo "${desc}: gremlin-server.yaml is ${state}, not named" >&2 + exit 1 + fi + class=$(rest_class "${dir}") + if [[ "${class}" != "org.apache.hugegraph.auth.StandardAuthenticator" ]]; then + echo "${desc}: rest-server.properties reads back [${class}]" >&2 + exit 1 + fi + if [[ "$(grep -c '^auth\.authenticator' "${dir}/conf/rest-server.properties")" != "1" ]]; then + echo "${desc}: auth.authenticator has more than one definition" >&2 + exit 1 + fi + # Parity has to survive the run, not just the files: a tree the script + # leaves one-sided must not still pass the guard that let it through. + if ! sides_agree "${dir}"; then + echo "${desc}: check_auth_sides rejects the tree enable-auth.sh left" >&2 + exit 1 + fi +} + +# An `authentication:` that belongs to another mapping is not the server's, so +# the script owns the whole of the Gremlin side and has to write it. +bootstrap_tree "${parity_dir}/nested" +printf '%s\n' 'host: 0.0.0.0' 'someFeature:' ' authentication:' \ + ' authenticator: com.example.Nested' \ + > "${parity_dir}/nested/conf/gremlin-server.yaml" +printf '%s\n' 'restserver.url=http://127.0.0.1:8080' \ + > "${parity_dir}/nested/conf/rest-server.properties" +accepted_then_both_sides "nested authentication mapping" "${parity_dir}/nested" +# The other feature keeps its own block untouched, and the block written for +# the server is the one at column 0. +grep -q '^authentication: {$' "${parity_dir}/nested/conf/gremlin-server.yaml" +grep -q '^ authentication:$' "${parity_dir}/nested/conf/gremlin-server.yaml" +grep -q '^ authenticator: com\.example\.Nested$' \ + "${parity_dir}/nested/conf/gremlin-server.yaml" + +# Both empty spellings, plus a whitespace value: each parses to the empty +# string, so each is the unconfigured side and has to be filled in place. +for empty in 'auth.authenticator=' 'auth.authenticator' 'auth.authenticator= '; do + bootstrap_tree "${parity_dir}/empty" + printf '%s\n' 'host: 0.0.0.0' > "${parity_dir}/empty/conf/gremlin-server.yaml" + printf '%s\n' "${empty}" 'unrelated=true' \ + > "${parity_dir}/empty/conf/rest-server.properties" + accepted_then_both_sides "empty definition [${empty}]" "${parity_dir}/empty" + # The placeholder is rewritten where it stood; unrelated content is kept. + grep -q '^unrelated=true$' "${parity_dir}/empty/conf/rest-server.properties" + [[ "$(head -1 "${parity_dir}/empty/conf/rest-server.properties")" == \ + 'auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' ]] +done + +# A value the operator did write is never a default's target. This tree is +# accepted because both sides already name the same class, and the script has +# to leave it alone rather than replace it with StandardAuthenticator. +bootstrap_tree "${parity_dir}/operator" +printf '%s\n' 'authentication:' ' authenticator: com.example.OperatorAuth' \ + > "${parity_dir}/operator/conf/gremlin-server.yaml" +printf '%s\n' 'auth.authenticator=com.example.OperatorAuth' \ + > "${parity_dir}/operator/conf/rest-server.properties" +if ! sides_agree "${parity_dir}/operator" >/dev/null 2>&1; then + echo "operator class tree: check_auth_sides refused" >&2 + exit 1 +fi +( cd "${parity_dir}/operator" && unset AUTHENTICATOR_CLASS && ./bin/enable-auth.sh ) +if [[ "$(rest_class "${parity_dir}/operator")" != "com.example.OperatorAuth" ]]; then + echo "operator class must survive the default write: got [$(rest_class "${parity_dir}/operator")]" >&2 + exit 1 +fi +if ! sides_agree "${parity_dir}/operator"; then + echo "operator class tree lost parity" >&2 + exit 1 +fi 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 639003f702..f64cec5dca 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 @@ -62,17 +62,9 @@ for candidate in "${PROPS_AWK:-}" "${BIN}/props.awk" "${TOP}/props.awk"; do done [[ -n "${PROPS_AWK:-}" ]] || fail "props.awk not found beside this script" -# Exit status of the reader is meaningful: 1 means the key has no definition, -# 2 means props.awk could not do its job. Only 1 is an acceptable answer here. -props_has() { - local status=0 - PROPS_MODE=has PROPS_KEY="$1" PROPS_FILE="$2" awk -f "${PROPS_AWK}" /dev/null || status=$? - if (( status > 1 )); then - fail "cannot read $2" - fi - return "${status}" -} - +# props_get is the only reader used here, and it treats any nonzero status from +# props.awk as an error: 2 means the file could not be read at all, which must +# not be mistaken for "the key is not there" and answered with a write. props_get() { local status=0 value value=$(PROPS_MODE=get PROPS_DECODED=1 PROPS_KEY="$1" PROPS_FILE="$2" \ @@ -94,6 +86,24 @@ props_set() { awk -f "${PROPS_AWK}" /dev/null || fail "cannot update $3" } +# Give `$3` its default `$2` for key `$1`, in place, unless it already has a +# value. Guarding with props_has and appending was not the same question: +# `auth.authenticator=` and a bare `auth.authenticator` line both parse to the +# empty string (measured against java.util.Properties, which also strips the +# trailing blanks of `auth.authenticator= `), so props_has reported them as +# answered and the append was skipped -- while the entrypoint's +# check_auth_sides, which asks for the value rather than the key, counted the +# same file as unconfigured. `loadAuthenticator("")` returns null, so REST then +# served without authentication next to a Gremlin that required it. +# +# props_set covers both shapes the guard had to split: with a definition +# present it replaces the first one where it stands (no duplicate for +# first-definition-wins to bury), with none present it appends. +ensure_rest_prop() { + [[ -n "$(props_get "$1" "$3")" ]] && return 0 + props_set "$1" "$2" "$3" +} + # make a backup BAK_CONF="$TOP/conf-bak" if [ ! -d "$BAK_CONF" ]; then @@ -106,11 +116,14 @@ if [ ! -d "$BAK_CONF" ]; then fail "cannot back up ${GRAPH_CONF}" fi -# The appends below are guarded per file and skip any file that already carries -# the property, so they are no-ops on a mounted config or a re-run. 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. +# Both writes below skip a side that already carries a real value, so they are +# no-ops on a mounted config or a re-run. 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. That is why the REST side goes +# through ensure_rest_prop rather than a presence guard plus an append: a +# presence guard also lets a defined-but-empty key count as answered, and the +# appended default would then be the definition the server never reads. # # Appended with `>>` rather than `sed -i '$a\...'`: GNU sed's `$` address never # matches when the file has no lines, so on an empty mounted config every append @@ -136,7 +149,14 @@ append_lines() { AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}" -if ! grep -Eq '^[[:blank:]]*authentication[[:blank:]]*:' "${CONF}/${GREMLIN_SERVER_CONF}"; then +# Only a column-0 `authentication` mapping is the Gremlin server's, which is the +# rule yamlscan.awk applies to decide the same thing for check_auth_sides. With +# `[[:blank:]]*` here the two disagreed on a config that nests `authentication` +# under another feature: the entrypoint read it as `none`, so parity held and it +# called this script, but this guard saw the nested key and skipped the append, +# writing the REST side only -- StandardAuthenticator on REST, TinkerPop's +# AllowAllAuthenticator on Gremlin. +if ! grep -Eq '^authentication[[:blank:]]*:' "${CONF}/${GREMLIN_SERVER_CONF}"; then append_lines "${CONF}/${GREMLIN_SERVER_CONF}" \ 'authentication: {' \ " authenticator: ${AUTHENTICATOR_CLASS}," \ @@ -145,13 +165,8 @@ if ! grep -Eq '^[[:blank:]]*authentication[[:blank:]]*:' "${CONF}/${GREMLIN_SERV '}' fi -if ! props_has "auth.authenticator" "${CONF}/${REST_SERVER_CONF}"; then - append_lines "${CONF}/${REST_SERVER_CONF}" "auth.authenticator=${AUTHENTICATOR_CLASS}" -fi - -if ! props_has "auth.graph_store" "${CONF}/${REST_SERVER_CONF}"; then - append_lines "${CONF}/${REST_SERVER_CONF}" 'auth.graph_store=hugegraph' -fi +ensure_rest_prop "auth.authenticator" "${AUTHENTICATOR_CLASS}" "${CONF}/${REST_SERVER_CONF}" +ensure_rest_prop "auth.graph_store" "hugegraph" "${CONF}/${REST_SERVER_CONF}" # Wrap the graph factory only when it really is the plain HugeFactory, which is # a question about the decoded value, so it goes through the same reader. From d5ccb942f669391448eecf220ac47e4e86d780a2 Mon Sep 17 00:00:00 2001 From: Adarsh Date: Thu, 24 Sep 2026 10:23:53 +0530 Subject: [PATCH 11/11] fix(docker): read the authenticator value the server actually reads Four findings from the re-review. Each was reproduced against this tree before anything changed, and each fix was then reverted to confirm its own test goes red -- 20 cases red before, 20 green after. yamlscan.awk - names_class() accepted any spelling of null except the lowercase one. snakeyaml resolves null case-insensitively, so `authenticator: NULL`, `Null` and `nUll` are the null node, and `!!null` states it outright; every one of them reported `named`, which is the one answer that cannot be forgiven: check_auth_sides then saw authentication on both sides and let REST enforce StandardAuthenticator over a Gremlin on AllowAllAuthenticator. An empty quoted scalar belongs here too -- the JVM hands loadAuthenticator the empty string and it returns null -- as does a value whose type comes from a tag this scanner cannot resolve, which is now refused rather than guessed at. A quoted "null" deliberately stays `named`: quoting makes it a string, and a class that does not exist fails loudly at startup instead of silently opening the server. Pinned as a case so a later tightening of the null rules has to move it on purpose. - scan_flow() lost the contents of a quoted value. Only keys accumulated inside the quotes, so {authenticator: "org.example.Auth"} arrived at commit_val() empty and read as nameless, refusing a valid mounted config before the server ever started. Quoted bytes now accumulate for a value as they do for a key, with the double-quoted backslash escape honoured so an inner quote does not end the scalar early. enable-auth.sh - The yaml half of the append guard now asks the same reader check_auth_sides uses. grep only ever matched the bare spelling, so an operator's `"authentication":` block read as absent and a second default block was appended beside it, after which the two servers resolve the key in opposite directions while REST keeps the authenticator the operator named. Measured before the change: one top-level key in, two out, in both the image and the tarball layout. The image gets yamlscan.awk from the install home, so PROPS_AWK-style discovery covers it; the plain release tarball carries no copy, and there the fallback grep at least knows the quoted spellings. Anything other than `none` means the mapping is there and is not this script's to duplicate. docker-entrypoint.sh - ACTUAL_BACKEND is compared against a literal, so it has to be read decoded. Against java.util.Properties as the oracle, a mounted `backend=hstore` is hstore to the JVM while the on-disk bytes are not, and the comparison then skipped wait-partition.sh and let startup continue before partitions were assigned. get_prop_decoded() is the new reader and the end-to-end case asserts wait-partition.sh is reached for the escaped spelling and still not reached for rocksdb. The other get_prop_encoded() callers were measured rather than swept along: the presence test at the parity check does not need decoding, because the encoded reader already trims trailing blanks the way Properties does, so the two readers agree there and only this literal comparison was wrong. Verified: yamlscan.awk over the null spellings, quoted flow values and nested flow cases (10 red / 20 ok after); enable-auth.sh in both layouts; the escaped-backend read against java.util.Properties on jdk17; both CI-wired shell suites at exit 0 with output identical to the pre-change baseline, and docker-entrypoint-test.sh reaching its PASS line; the wait-partition case shown red by restoring get_prop_encoded at that one call site. Not verified here: mawk, which is not installed on this host; CR-only and CRLF gremlin-server.yaml, which MSYS text mode cannot observe; snakeyaml itself, never executed, so the null spellings follow the resolver's documented behaviour rather than a run of it, and the refusal of an unresolvable tag is a judgement call about the safe direction rather than a measurement; and no Docker daemon, so no image build. --- .../docker/docker-entrypoint-test.sh | 46 +++++++ .../docker/docker-entrypoint.sh | 21 ++- .../docker/test/test-docker-entrypoint.sh | 128 +++++++++++++++++- .../hugegraph-dist/docker/yamlscan.awk | 50 ++++++- .../src/assembly/static/bin/enable-auth.sh | 53 ++++++-- 5 files changed, 281 insertions(+), 17 deletions(-) diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh index 2eeffbad5f..3aceea1aff 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh @@ -58,6 +58,7 @@ printf 'called\n' >> ./docker/enable-auth-calls EOF cat > "${TEST_HOME}/bin/wait-partition.sh" <<'EOF' #!/usr/bin/env bash +printf 'called\n' >> ./docker/wait-partition-calls exit 0 EOF cat > "${TEST_HOME}/bin/wait-storage.sh" <<'EOF' @@ -374,4 +375,49 @@ rm -f "${TEST_HOME}/docker/init_complete" bash ./docker-entrypoint.sh ) +# ── The stabilization check follows the backend the JVM actually loaded ── +# ACTUAL_BACKEND is compared against a literal, so it has to be the decoded +# value. A mounted hugegraph.properties may spell the word with a unicode +# escape for the s, which java.util.Properties hands the server as hstore; +# reading the on-disk escaping instead compared something else to hstore, +# skipped wait-partition.sh, and let startup continue before the partitions +# were assigned. bs is the backslash, taken from its code point rather than +# written here: printf '%c' 92 hands back the digit 9, which would have built a +# fixture holding a different word than the one being decoded. +bs=$(awk 'BEGIN { printf "%c", 92 }') +if [[ "${#bs}" != 1 || "$(printf '%d' "'${bs}")" != 92 ]]; then + echo "this host did not yield a backslash for code point 92" >&2 + exit 1 +fi +touch "${TEST_HOME}/docker/init_complete" +rm -f "${TEST_HOME}/docker/wait-partition-calls" +printf '%s\n' "backend=h${bs}u0073tore" 'pd.peers=pd:8686' \ + > "${TEST_HOME}/conf/graphs/hugegraph.properties" +if [[ "$(head -n 1 "${TEST_HOME}/conf/graphs/hugegraph.properties")" != \ + "backend=h${bs}u0073tore" ]]; then + echo "the fixture has to hold the escaped bytes, not the decoded word" >&2 + exit 1 +fi +( + cd "${TEST_HOME}" + bash ./docker-entrypoint.sh +) +if [[ ! -s "${TEST_HOME}/docker/wait-partition-calls" ]]; then + echo "an escaped hstore backend must still reach wait-partition.sh" >&2 + exit 1 +fi +# The other half: this is a read that follows the server, not a switch that +# simply always waits. +rm -f "${TEST_HOME}/docker/wait-partition-calls" +printf '%s\n' 'backend=rocksdb' 'pd.peers=pd:8686' \ + > "${TEST_HOME}/conf/graphs/hugegraph.properties" +( + cd "${TEST_HOME}" + bash ./docker-entrypoint.sh +) +if [[ -e "${TEST_HOME}/docker/wait-partition-calls" ]]; then + echo "wait-partition.sh ran for a rocksdb backend" >&2 + exit 1 +fi + echo "PASS: Docker entrypoint configures HStore discovery and authentication" diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh index 93d77d4fc2..dcb39c4f1f 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh @@ -106,6 +106,17 @@ get_prop_encoded() { awk -f "${PROPS_AWK}" /dev/null } +# The value as java.util.Properties hands it to the server, escapes resolved. +# Compare against this, not the on-disk bytes: `backend=h\u0073tore` is a legal +# spelling of hstore that the JVM reads as hstore and a string compare against +# the raw text does not. +get_prop_decoded() { + local key="$1" file="$2" + + PROPS_MODE=get PROPS_DECODED=1 PROPS_KEY="${key}" PROPS_FILE="${file}" \ + awk -f "${PROPS_AWK}" /dev/null +} + # What the top-level authentication mapping of gremlin-server.yaml says about # authentication, as one of three states: none, named, nameless. # @@ -333,9 +344,13 @@ fi # Post-startup cluster stabilization check (hstore only — rocksdb has no partitions) # Read through props.awk so a mounted config using the `:` or bare-whitespace # separator is seen at all, and first-definition-wins matches HugeConfig; the -# grep this replaces only ever accepted `=`. Trailing whitespace is dropped -# here rather than in the reader, which reports the on-disk bytes verbatim. -ACTUAL_BACKEND=$(get_prop_encoded "backend" "${GRAPH_CONF}" | tr -d '[:space:]' || true) +# grep this replaces only ever accepted `=`. Decoded, because this is compared +# against a literal: the JVM reads `backend=h\u0073tore` as hstore while the +# on-disk bytes are not that string, and the comparison deciding to skip +# wait-partition.sh is how startup continued before partitions were assigned. +# Trailing whitespace is dropped here rather than in the reader, which reports +# the value verbatim apart from the escapes java.util.Properties resolves. +ACTUAL_BACKEND=$(get_prop_decoded "backend" "${GRAPH_CONF}" | tr -d '[:space:]' || true) if [[ "${ACTUAL_BACKEND}" == "hstore" ]]; then STORE_REST="${STORE_REST:-store:8520}" export STORE_REST 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 411c711a6e..63317ac7ee 100644 --- a/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh @@ -28,7 +28,7 @@ trap 'rm -rf "${test_dir}"' EXIT # 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 \ - yaml_auth_state check_auth_sides; do + get_prop_decoded yaml_auth_state check_auth_sides; do eval "$(awk -v fn="${fn}" ' index($0, fn "() {") == 1 { capture = 1 } capture { print } @@ -45,11 +45,16 @@ export PROPS_AWK YAMLSCAN # enable-auth.sh reads and writes .properties through props.awk, which the # release assembly packages in the same bin/ directory. A test tree that runs # the script therefore has to carry both, or it is not the layout it ships in. +# yamlscan.awk goes one level up, in the install home, exactly where the +# Dockerfile puts it: the guard in enable-auth.sh has to answer the Gremlin +# question with the same reader check_auth_sides uses, so a tree without it +# would be testing the tarball fallback rather than the image. install_enable_auth() { local dir="$1" mkdir -p "${dir}/bin" cp "${static_bin}/enable-auth.sh" "${dir}/bin/enable-auth.sh" cp "${static_bin}/props.awk" "${dir}/bin/props.awk" + cp "${docker_dir}/yamlscan.awk" "${dir}/yamlscan.awk" chmod +x "${dir}/bin/enable-auth.sh" } @@ -937,6 +942,47 @@ yaml_case nameless "sibling key closes the mapping" \ ' handler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler' \ 'metrics:' \ ' authenticator: org.apache.hugegraph.auth.StandardAuthenticator' +# What snakeyaml resolves to null names no class, and it resolves null +# case-insensitively, so NULL, Null and nUll are the refusal case just as null +# is. An explicit !!null says it outright, and an empty quoted scalar is the +# empty string, for which loadAuthenticator returns null. Reporting any of +# these as named is the one direction that cannot be forgiven: REST would +# enforce while Gremlin ran on AllowAllAuthenticator. +for nullish in 'null' 'NULL' 'Null' 'nUll' '~' '!!null' '!!null ~' '""' "''"; do + yaml_case nameless "authenticator set to the null spelling [${nullish}]" \ + 'authentication:' \ + " authenticator: ${nullish}" +done +# The same values in a flow mapping, where the reader has to reach the value at +# all: a quoted class name used to arrive empty, which refused a valid mounted +# config before the server started. +yaml_case named "unquoted class in a flow mapping" \ + 'authentication: {authenticator: org.apache.hugegraph.auth.StandardAuthenticator}' +yaml_case named "double quoted class in a flow mapping" \ + 'authentication: {authenticator: "org.apache.hugegraph.auth.StandardAuthenticator"}' +yaml_case named "single quoted class in a flow mapping" \ + "authentication: {authenticator: 'org.apache.hugegraph.auth.StandardAuthenticator'}" +yaml_case named "double quoted class between flow siblings" \ + 'authentication: {config: {tokens: conf/rest-server.properties}, authenticator: "org.apache.hugegraph.auth.StandardAuthenticator"}' +# Quoting a scalar makes it a string rather than the null node, so "null" names +# a class the server fails to load loudly at startup; that is not the silent +# no-authenticator state the plain spellings above are. Pinned so a later +# tightening of the null rules cannot move it without saying so here. +yaml_case named "quoted null is a string, not the null node" \ + 'authentication:' \ + ' authenticator: "null"' +# A nested mapping is still the config map even when the value inside it is +# quoted, and an empty quoted scalar is the empty string the server reads as no +# authenticator. +yaml_case nameless "class nested under a flow config, quoted" \ + 'authentication: {config: {authenticator: "org.apache.hugegraph.auth.StandardAuthenticator"}}' +yaml_case nameless "flow value that is an empty quoted string" \ + 'authentication: {authenticator: ""}' +# A tag, not the text, decides the type of a scalar, and a type this scanner +# cannot resolve is refused rather than guessed at. +yaml_case nameless "explicit str tag, a type this scanner cannot resolve" \ + 'authentication:' \ + ' authenticator: !!str org.apache.hugegraph.auth.StandardAuthenticator' # ── Mounted one-sided config is refused with no PASSWORD ─────────────── # check_auth_sides used to run only inside the PASSWORD branch, so a mounted @@ -1106,3 +1152,83 @@ if ! sides_agree "${parity_dir}/operator"; then echo "operator class tree lost parity" >&2 exit 1 fi + +# ── The Gremlin guard and check_auth_sides have to read the same key ──── +# enable-auth.sh skips the yaml append when the file already carries a +# top-level authentication mapping. A grep that only knew the bare spelling +# called an operator's "authentication": block absent and appended a second +# one beside it, after which the two servers resolve the key in opposite +# directions while REST keeps the authenticator the operator named. +top_level_auth_keys() { + local file="$1" sq="'" + grep -Ec "^[\"${sq}]?authentication[\"${sq}]?[[:blank:]]*:" "${file}" +} + +quoted_key_tree() { # + local dir="$1" with_scan="$2" + bootstrap_tree "${dir}" + [[ "${with_scan}" == "yes" ]] || rm -f "${dir}/yamlscan.awk" + printf '%s\n' 'host: 0.0.0.0' '"authentication":' \ + ' authenticator: com.example.OperatorAuth' > "${dir}/conf/gremlin-server.yaml" + printf '%s\n' 'restserver.url=http://127.0.0.1:8080' \ + 'auth.authenticator=com.example.OperatorAuth' \ + > "${dir}/conf/rest-server.properties" + if ! sides_agree "${dir}" >/dev/null 2>&1; then + echo "quoted top-level key (${with_scan}): check_auth_sides refused" >&2 + exit 1 + fi + ( cd "${dir}" && unset AUTHENTICATOR_CLASS && ./bin/enable-auth.sh ) || { + echo "quoted top-level key (${with_scan}): enable-auth.sh failed" >&2 + exit 1 + } + if [[ "$(top_level_auth_keys "${dir}/conf/gremlin-server.yaml")" != "1" ]]; then + echo "quoted top-level key (${with_scan}): the append duplicated the" \ + "operator block, got $(top_level_auth_keys "${dir}/conf/gremlin-server.yaml")" >&2 + cat "${dir}/conf/gremlin-server.yaml" >&2 + exit 1 + fi + if [[ "$(gremlin_state "${dir}")" != "named" ]]; then + echo "quoted top-level key (${with_scan}): yaml is no longer named" >&2 + exit 1 + fi + if [[ "$(rest_class "${dir}")" != "com.example.OperatorAuth" ]]; then + echo "quoted top-level key (${with_scan}): REST lost the operator class" >&2 + exit 1 + fi +} + +# The image layout, where yamlscan.awk sits in the install home, so the guard +# asks the same reader the entrypoint does. +quoted_key_tree "${parity_dir}/quoted-key" yes +# The plain release tarball carries no yamlscan.awk; the fallback has to keep +# the same answer for the question it can honestly settle on its own. +quoted_key_tree "${parity_dir}/quoted-key-tarball" no + +# ── A value compared to a literal is the decoded value, not the bytes ─── +# wait-partition.sh is skipped unless ACTUAL_BACKEND reads hstore. The JVM +# resolves `backend=h\u0073tore` to hstore, so a reader that hands back the +# on-disk escaping does not see the backend that is actually running, and +# startup continues before the partitions are assigned. +escaped_backend="${test_dir}/backend-escape.properties" +# %s, not the format string: printf resolves \u0073 in a format itself and would +# write the decoded word, which is the very thing this case has to hand the +# reader. The next assertion is the guard rail against that happening silently. +printf '%s\n' 'backend=h\u0073tore' > "${escaped_backend}" +if [[ "$(tr -d '\n' < "${escaped_backend}")" != 'backend=h\u0073tore' ]]; then + echo "fixture must hold the escaped bytes on disk, got [$(cat "${escaped_backend}")]" >&2 + exit 1 +fi +if [[ "$(get_prop_encoded backend "${escaped_backend}")" == "hstore" ]]; then + echo "the encoded reader is expected to report the on-disk escaping" >&2 + exit 1 +fi +if [[ "$(get_prop_decoded backend "${escaped_backend}")" != "hstore" ]]; then + echo "decoded read of an escaped backend gave" \ + "[$(get_prop_decoded backend "${escaped_backend}")]" >&2 + exit 1 +fi +# The ordinary spelling is unaffected, so this is not decode-instead-of-read. +plain_backend="${test_dir}/backend-plain.properties" +printf 'backend=hstore\n' > "${plain_backend}" +[[ "$(get_prop_decoded backend "${plain_backend}")" == "hstore" ]] +[[ "$(get_prop_encoded backend "${plain_backend}")" == "hstore" ]] diff --git a/hugegraph-server/hugegraph-dist/docker/yamlscan.awk b/hugegraph-server/hugegraph-dist/docker/yamlscan.awk index 4c127eb7c5..9daf1b15c0 100644 --- a/hugegraph-server/hugegraph-dist/docker/yamlscan.awk +++ b/hugegraph-server/hugegraph-dist/docker/yamlscan.awk @@ -124,10 +124,37 @@ function split_pair(s, i, n, c, q) { function names_authenticator(k) { return unquote(k) == "authenticator" } -# An authenticator entry only counts when it actually names a class. -function names_class(v) { +# An authenticator entry only counts when it actually names a class, and the +# answer has to be what snakeyaml hands the server rather than what the bytes +# look like. The unsafe direction is `named` for a config that leaves Gremlin +# on AllowAllAuthenticator while REST enforces, so anything this scanner cannot +# resolve to a class is refused instead of guessed at: +# +# - a plain scalar that resolves to null in any spelling, and YAML resolves +# null case-insensitively (null, Null, NULL, nUll) as well as to ~, names +# no class; +# - a leading `!` makes the tag, not the text, decide the type: !!null is the +# explicit spelling of empty and every other tag is a type not resolvable +# here, so neither counts; +# - a quoted scalar is a string and never null, but `""` and the empty single +# quoted form are the empty string, and loadAuthenticator("") returns null, +# which is the same no-authenticator state; +# - an unterminated quote is not a scalar at all. +function names_class(v, first, last, body) { v = trim(v) - return v != "" && v != "null" && v != "~" + if (v == "") return 0 + first = substr(v, 1, 1) + if (first == "!") return 0 + if (first == apos() || first == dquo()) { + if (length(v) < 2) return 0 + last = substr(v, length(v), 1) + if (last != first) return 0 + body = trim(substr(v, 2, length(v) - 2)) + return body != "" + } + if (v == "~") return 0 + if (tolower(v) == "null") return 0 + return 1 } # Report and stop. Output happens in END only, because awk runs END after @@ -139,19 +166,32 @@ function finish(r) { RESULT = r; exit } # a nested mapping under `config` invisible to it. FSET records a direct # authenticator that names a class. Returns 1 once the outermost collection # has closed. -function scan_flow(s, i, n, c, q) { +function scan_flow(s, i, n, c, q, esc) { n = length(s) q = "" + esc = 0 for (i = 1; i <= n; i++) { c = substr(s, i, 1) if (q != "") { + # Every byte inside the quotes belongs to the scalar, delimiters + # included; unquote and names_class take the quotes off. Dropping + # the value here is what made {authenticator: "org.A"} read as + # nameless and refuse a valid mounted config. A backslash escapes + # the next byte in a double quoted scalar only -- in a single + # quoted one the way out is a doubled quote, which this loop + # already gets right because the first one closes and the next + # reopens, and the pair still counts as content. if (FST == "key") CUR = CUR c - if (c == q) q = "" + else if (FST == "val") CUR_VAL = CUR_VAL c + if (esc) esc = 0 + else if (q == dquo() && c == "\\") esc = 1 + else if (c == q) q = "" continue } if (is_quote(c)) { q = c if (FST == "key") CUR = CUR c + else if (FST == "val") CUR_VAL = CUR_VAL c continue } if (c == "{" || c == "[") { 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 f64cec5dca..e090530296 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 @@ -52,8 +52,8 @@ fail() { # to be handed a carriage return for is now handled by the reader itself. # # props.awk is packaged in this same bin/ directory by the release assembly, so -# it is present in the tarball and in the image; the entrypoint also exports -# PROPS_AWK when it calls this script. +# it is present in the tarball and in the image. PROPS_AWK lets a caller point +# at a different copy; the entrypoint reads that same variable for itself. for candidate in "${PROPS_AWK:-}" "${BIN}/props.awk" "${TOP}/props.awk"; do if [[ -n "${candidate}" && -f "${candidate}" ]]; then PROPS_AWK="${candidate}" @@ -62,6 +62,20 @@ for candidate in "${PROPS_AWK:-}" "${BIN}/props.awk" "${TOP}/props.awk"; do done [[ -n "${PROPS_AWK:-}" ]] || fail "props.awk not found beside this script" +# The Gremlin half of the decision below has to be made by the same reader the +# entrypoint uses. yamlscan.awk is not in bin/: the Dockerfile places it in the +# install home, one above this script, which is where the image layout is +# mirrored in the test tree; PROPS_AWK and YAMLSCAN_AWK cover a caller that +# keeps it elsewhere. The release tarball carries no copy at all, so the +# fallback below has to answer on its own. +YAMLSCAN="" +for candidate in "${YAMLSCAN_AWK:-}" "${TOP}/yamlscan.awk" "${BIN}/yamlscan.awk"; do + if [[ -n "${candidate}" && -f "${candidate}" ]]; then + YAMLSCAN="${candidate}" + break + fi +done + # props_get is the only reader used here, and it treats any nonzero status from # props.awk as an error: 2 means the file could not be read at all, which must # not be mistaken for "the key is not there" and answered with a write. @@ -149,14 +163,37 @@ append_lines() { AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}" +# Does the Gremlin config already carry a top-level `authentication` mapping? +# This is the same question check_auth_sides answers, so it has to go to the same +# reader: a mapping is the server's only at column 0, comment text is not +# content, and the key may be quoted. grep asks it differently -- it sees only +# the bare spelling, so an operator's `"authentication":` block read as absent +# and a second default block was appended beside it, after which the two servers +# can resolve the key in opposite directions while REST keeps its existing +# authenticator. Anything other than `none` means a mapping is there and the +# append is not this script's to make. +gremlin_has_auth_block() { + local file="$1" state + [[ -f "${file}" ]] || return 1 + if [[ -n "${YAMLSCAN}" ]]; then + state=$(awk -f "${YAMLSCAN}" "${file}") || fail "cannot read ${file}" + [[ "${state}" != "none" ]] + return + fi + # No parser in this layout (the plain release tarball). Match what grep can + # honestly answer here: a column-0 key in either quote style or none. The + # nested-mapping and comment cases are the ones that need the real reader, + # and the image, where the entrypoint runs this script, always has it. + grep -Eq "^[\"']?authentication[\"']?[[:blank:]]*:" "${file}" +} + # Only a column-0 `authentication` mapping is the Gremlin server's, which is the # rule yamlscan.awk applies to decide the same thing for check_auth_sides. With -# `[[:blank:]]*` here the two disagreed on a config that nests `authentication` -# under another feature: the entrypoint read it as `none`, so parity held and it -# called this script, but this guard saw the nested key and skipped the append, -# writing the REST side only -- StandardAuthenticator on REST, TinkerPop's -# AllowAllAuthenticator on Gremlin. -if ! grep -Eq '^authentication[[:blank:]]*:' "${CONF}/${GREMLIN_SERVER_CONF}"; then +# a guard that disagreed on nesting, the entrypoint read the file as `none`, so +# parity held and it called this script, but the guard saw the nested key and +# skipped the append, writing the REST side only -- StandardAuthenticator on +# REST, TinkerPop's AllowAllAuthenticator on Gremlin. +if ! gremlin_has_auth_block "${CONF}/${GREMLIN_SERVER_CONF}"; then append_lines "${CONF}/${GREMLIN_SERVER_CONF}" \ 'authentication: {' \ " authenticator: ${AUTHENTICATOR_CLASS}," \