fix(security): validate bundle and clean runtime secrets
This commit is contained in:
@@ -47,3 +47,11 @@ that neither secret values nor bundle/file metadata are inherited by the Pi chil
|
||||
|
||||
The rotation helper retains its old/new scratch-file CLI contract; smoke tests keep those files
|
||||
outside Compose and mount only the bundle.
|
||||
|
||||
## Whole-branch review fixes
|
||||
|
||||
- `core-entrypoint.sh` validates `THT_SECRETS_FILE` fail-closed before optional lookups; malformed,
|
||||
duplicate, unknown, oversized, or overlong bundles stop startup with sanitized diagnostics.
|
||||
- Runtime password files are cleaned after child exit via signal forwarding and `wait`, rather
|
||||
than being orphaned by `exec`.
|
||||
- The shell loader accepts CRLF bundles (Windows/Notepad) consistently with the TypeScript loader.
|
||||
|
||||
@@ -23,12 +23,11 @@ read_secret_file() {
|
||||
cat "$1"
|
||||
}
|
||||
|
||||
# Read one value from the deployment bundle without putting the bundle itself in
|
||||
# a service environment. The parser is deliberately strict: one KEY=VALUE per
|
||||
# line, no duplicate keys, no unknown syntax, and no whitespace in credentials.
|
||||
read_bundle_secret() {
|
||||
# Validate the bundle without printing any value. Keep this parser aligned with
|
||||
# the backend loader: comments/blank lines are allowed, while syntax, allowlist,
|
||||
# duplicates, empty values, file size, and line size are fail-closed.
|
||||
validate_bundle() {
|
||||
bundle_path=$1
|
||||
bundle_key=$2
|
||||
if [ -L "$bundle_path" ] || [ ! -f "$bundle_path" ] || [ ! -r "$bundle_path" ] || [ ! -s "$bundle_path" ]; then
|
||||
echo "secret bundle must be a readable, non-empty regular file" >&2
|
||||
return 2
|
||||
@@ -38,20 +37,46 @@ read_bundle_secret() {
|
||||
/run/secrets/*:444|/run/secrets/*:400|/run/secrets/*:600|*:600|*:400) ;;
|
||||
*) echo "secret bundle must have mode 0600 or stricter (Docker secrets may be 0444)" >&2; return 2 ;;
|
||||
esac
|
||||
case "$bundle_key" in
|
||||
THT_[A-Z0-9_]*|PI_PROVIDER_API_KEY) ;;
|
||||
*) echo "invalid secret bundle key" >&2; return 2 ;;
|
||||
esac
|
||||
value=$(awk -v wanted="$bundle_key" '
|
||||
size=$(stat -c '%s' "$bundle_path" 2>/dev/null || stat -f '%z' "$bundle_path" 2>/dev/null) || return 2
|
||||
if [ "$size" -gt 65536 ]; then
|
||||
echo "secret bundle exceeds the 64KiB limit" >&2
|
||||
return 2
|
||||
fi
|
||||
awk '
|
||||
{ sub(/\r$/, "", $0) }
|
||||
length($0) > 16384 { exit 9 }
|
||||
/^[[:space:]]*$/ || /^[[:space:]]*#/ { next }
|
||||
/^[A-Z][A-Z0-9_]*=/ {
|
||||
key=$0; sub(/=.*/, "", key)
|
||||
val=$0; sub(/^[^=]*=/, "", val)
|
||||
if (key !~ /^(THT_MODEL_API_KEY|THT_DWH_API_KEY|THT_VEC_API_KEY|THT_VEC_WRITE_API_KEY|THT_CA|THT_SSL_CA|THT_VECTOR_BOOTSTRAP_PASSWORD|THT_VECTOR_MIGRATOR_PASSWORD|THT_VECTOR_READER_PASSWORD|THT_VECTOR_WRITER_PASSWORD|PI_PROVIDER_API_KEY)$/) exit 6
|
||||
if (val == "") exit 7
|
||||
if (++seen[key] > 1) exit 8
|
||||
if (val == "" || ++seen[key] > 1) exit 7
|
||||
next
|
||||
}
|
||||
{ exit 4 }
|
||||
' "$bundle_path" || {
|
||||
echo "secret bundle syntax is invalid" >&2
|
||||
return 2
|
||||
}
|
||||
}
|
||||
|
||||
# Read one value from the deployment bundle without putting the bundle itself in
|
||||
# a service environment. Values selected for credentials must contain no spaces.
|
||||
read_bundle_secret() {
|
||||
bundle_path=$1
|
||||
bundle_key=$2
|
||||
validate_bundle "$bundle_path" || return
|
||||
case "$bundle_key" in
|
||||
THT_[A-Z0-9_]*|PI_PROVIDER_API_KEY) ;;
|
||||
*) echo "invalid secret bundle key" >&2; return 2 ;;
|
||||
esac
|
||||
value=$(awk -v wanted="$bundle_key" '
|
||||
{ sub(/\r$/, "", $0) }
|
||||
/^[[:space:]]*$/ || /^[[:space:]]*#/ { next }
|
||||
/^[A-Z][A-Z0-9_]*=/ {
|
||||
key=$0; sub(/=.*/, "", key)
|
||||
val=$0; sub(/^[^=]*=/, "", val)
|
||||
if (key == wanted) {
|
||||
if (found) exit 3
|
||||
found=1; print val
|
||||
}
|
||||
next
|
||||
|
||||
@@ -36,8 +36,23 @@ cleanup_secret_tmp() {
|
||||
}
|
||||
trap cleanup_secret_tmp EXIT HUP INT TERM
|
||||
|
||||
run_child() {
|
||||
"$@" &
|
||||
child_pid=$!
|
||||
forward_signal() { kill -TERM "$child_pid" 2>/dev/null || true; }
|
||||
trap forward_signal HUP INT TERM
|
||||
wait "$child_pid"
|
||||
status=$?
|
||||
trap - HUP INT TERM
|
||||
return "$status"
|
||||
}
|
||||
|
||||
bundle=${THT_SECRETS_FILE:-}
|
||||
if [ -n "$bundle" ] && [ -r "$bundle" ]; then
|
||||
if [ -n "$bundle" ]; then
|
||||
if ! validate_bundle "$bundle" >/dev/null 2>&1; then
|
||||
echo "THT_SECRETS_FILE points to an invalid secret bundle" >&2
|
||||
exit 2
|
||||
fi
|
||||
# REST adapters consume these values while the harness is running. They are
|
||||
# loaded here (before `tht` starts), not only in the backend/Pi process.
|
||||
load_bundle_env() {
|
||||
@@ -76,21 +91,21 @@ fi
|
||||
case "${1:-server}" in
|
||||
server)
|
||||
shift || true
|
||||
exec node /app/backend/dist/server.js "$@"
|
||||
run_child node /app/backend/dist/server.js "$@"
|
||||
;;
|
||||
doctor)
|
||||
shift
|
||||
exec tht doctor "$@"
|
||||
run_child tht doctor "$@"
|
||||
;;
|
||||
preprocess)
|
||||
shift
|
||||
exec tht preprocess "$@"
|
||||
run_child tht preprocess "$@"
|
||||
;;
|
||||
tht)
|
||||
shift
|
||||
exec tht "$@"
|
||||
run_child tht "$@"
|
||||
;;
|
||||
*)
|
||||
exec tht "$@"
|
||||
run_child tht "$@"
|
||||
;;
|
||||
esac
|
||||
|
||||
@@ -74,6 +74,22 @@ if grep -q 'must-not-leak' "$tmp/legacy-model.err"; then
|
||||
echo "legacy model credential leaked through entrypoint diagnostics" >&2
|
||||
exit 1
|
||||
fi
|
||||
printf 'THT_VECTOR_READER_PASSWORD=one\nTHT_VECTOR_READER_PASSWORD=two\n' >"$tmp/invalid-bundle"
|
||||
chmod 0600 "$tmp/invalid-bundle"
|
||||
if THT_SECRETS_FILE="$tmp/invalid-bundle" ./docker/core-entrypoint.sh doctor \
|
||||
>"$tmp/invalid-bundle.out" 2>"$tmp/invalid-bundle.err"; then
|
||||
echo "entrypoint accepted an invalid secret bundle" >&2
|
||||
exit 1
|
||||
fi
|
||||
grep -q 'THT_SECRETS_FILE points to an invalid secret bundle' "$tmp/invalid-bundle.err"
|
||||
if grep -q 'THT_VECTOR_READER_PASSWORD' "$tmp/invalid-bundle.err"; then
|
||||
echo "invalid bundle diagnostics leaked key material" >&2
|
||||
exit 1
|
||||
fi
|
||||
before_tmp=$(find "${TMPDIR:-/tmp}" -maxdepth 1 -type d -name 'thothii-secrets.*' -print | sort)
|
||||
THT_SECRETS_FILE="$bundle" ./docker/core-entrypoint.sh doctor >/dev/null 2>&1 || true
|
||||
after_tmp=$(find "${TMPDIR:-/tmp}" -maxdepth 1 -type d -name 'thothii-secrets.*' -print | sort)
|
||||
test "$before_tmp" = "$after_tmp"
|
||||
if grep -Eq 'THT_VECTOR_(BOOTSTRAP|MIGRATOR|READER|WRITER)_PASSWORD_FILE|target: vector_(bootstrap|migrator|reader|writer)_password|dwh_api_key|model_api_key|THT_[A-Z0-9_]+_SECRET_FILE' "$tmp/production.yaml"; then
|
||||
echo "production external config contains local direct vector secrets" >&2
|
||||
exit 1
|
||||
|
||||
@@ -16,11 +16,14 @@ printf 'owner-readonly' >"$tmp/readonly"
|
||||
printf 'too-open' >"$tmp/open"
|
||||
printf '# comment\n\nTHT_VECTOR_READER_PASSWORD=reader\nTHT_VECTOR_WRITER_PASSWORD=writer\n' >"$tmp/bundle"
|
||||
printf 'THT_VECTOR_READER_PASSWORD=reader\nTHT_VECTOR_WRITER_PASSWORD=writer\nTHT_DWH_API_KEY=one\nTHT_DWH_API_KEY=two\n' >"$tmp/duplicate-bundle"
|
||||
printf 'THT_VECTOR_READER_PASSWORD=reader\r\nTHT_VECTOR_WRITER_PASSWORD=writer\r\n' >"$tmp/crlf-bundle"
|
||||
awk 'BEGIN { printf "THT_VECTOR_READER_PASSWORD="; for (i = 1; i <= 16385; i++) printf "x"; print "" }' >"$tmp/long-line-bundle"
|
||||
awk 'BEGIN { for (i = 1; i <= 70000; i++) print "# filler" }' >"$tmp/large-bundle"
|
||||
chmod 0600 "$tmp/valid"
|
||||
chmod 0444 "$tmp/docker"
|
||||
chmod 0400 "$tmp/readonly"
|
||||
chmod 0640 "$tmp/open"
|
||||
chmod 0600 "$tmp/bundle" "$tmp/duplicate-bundle"
|
||||
chmod 0600 "$tmp/bundle" "$tmp/duplicate-bundle" "$tmp/crlf-bundle" "$tmp/long-line-bundle" "$tmp/large-bundle"
|
||||
|
||||
for invalid in empty newline space; do
|
||||
if validate_secret_file "$tmp/$invalid" "$invalid" >/dev/null 2>&1; then
|
||||
@@ -40,9 +43,16 @@ if validate_secret_file "$tmp/open" open >/dev/null 2>&1; then
|
||||
fi
|
||||
test "$(read_secret_file "$tmp/valid" valid)" = "safe-quoted-'-dollar-$"
|
||||
test "$(read_bundle_secret "$tmp/bundle" THT_VECTOR_READER_PASSWORD)" = reader
|
||||
test "$(read_bundle_secret "$tmp/crlf-bundle" THT_VECTOR_READER_PASSWORD)" = reader
|
||||
if read_bundle_secret "$tmp/duplicate-bundle" THT_VECTOR_READER_PASSWORD >/dev/null 2>&1; then
|
||||
echo "secret policy accepted a duplicate unrelated bundle key" >&2
|
||||
exit 1
|
||||
fi
|
||||
for invalid_bundle in long-line-bundle large-bundle; do
|
||||
if read_bundle_secret "$tmp/$invalid_bundle" THT_VECTOR_READER_PASSWORD >/dev/null 2>&1; then
|
||||
echo "secret policy accepted oversized $invalid_bundle" >&2
|
||||
exit 1
|
||||
fi
|
||||
done
|
||||
|
||||
echo "shared vector secret policy contracts passed."
|
||||
|
||||
Reference in New Issue
Block a user