From 7069e87e3b5d4cd3290f20e927f876237529571e Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:57:49 +0800 Subject: [PATCH] fix(skill): validate derived registry hosts Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../tests/web-base-path-nginx-smoke-test.sh | 65 +++++++++++++++++-- web/docker-entrypoint.d/30-runtime-config.sh | 11 +++- web/nginx.conf.template | 2 + 3 files changed, 72 insertions(+), 6 deletions(-) diff --git a/scripts/tests/web-base-path-nginx-smoke-test.sh b/scripts/tests/web-base-path-nginx-smoke-test.sh index aac6b737..0ff06504 100755 --- a/scripts/tests/web-base-path-nginx-smoke-test.sh +++ b/scripts/tests/web-base-path-nginx-smoke-test.sh @@ -19,7 +19,7 @@ port=18080 tmp=$(mktemp -d) cleanup() { - docker rm -f "$name" "$name-fixed" "$name-default" >/dev/null 2>&1 || true + docker rm -f "$name" "$name-fixed" "$name-default" "$name-trusted" >/dev/null 2>&1 || true rm -rf "$tmp" } trap cleanup EXIT @@ -124,6 +124,14 @@ if [ "$cache_control" != 'no-cache' ]; then exit 1 fi +# An explicit URL is authoritative and must not interpolate a hostile request Host. +explicit_hostile=$(curl -fsS -H 'Host: evil.example;echo_injected' "$base/skillhub/install/skillhub.md") +printf '%s' "$explicit_hostile" | grep -F 'The primary registry for this guide is `https://skill.example.com/skillhub`.' >/dev/null +if printf '%s' "$explicit_hostile" | grep -F 'echo_injected' >/dev/null; then + echo 'explicit Agent guide must not interpolate the request Host' >&2 + exit 1 +fi + docker rm -f "$name" >/dev/null 2>&1 || true # With no explicit public URL, the guide must derive the registry from the @@ -139,7 +147,7 @@ docker run -d --name "$name_default" \ -p "$port_default:80" \ -e SKILLHUB_API_UPSTREAM=http://127.0.0.1:9 \ -e SKILLHUB_TRUST_FORWARDED_PROTO=false \ - -e SKILLHUB_WEB_BASE_PATH=/ \ + -e SKILLHUB_WEB_BASE_PATH=/skillhub/ \ -v "$default_html:/usr/share/nginx/html" \ -v "$ROOT_DIR/web/nginx.conf.template:/etc/nginx/templates/default.conf.template:ro" \ -v "$entrypoint_d/20-base-path.sh:/docker-entrypoint.d/20-base-path.sh:ro" \ @@ -157,14 +165,63 @@ until curl -fsS -o /dev/null "$default_base/nginx-health" 2>/dev/null; do fi sleep 1 done -default_guide=$(curl -fsS "$default_base/install/skillhub.md") -printf '%s' "$default_guide" | grep -F "The primary registry for this guide is \`$default_base\`." >/dev/null +default_guide=$(curl -fsS "$default_base/skillhub/install/skillhub.md") +printf '%s' "$default_guide" | grep -F "The primary registry for this guide is \`$default_base/skillhub\`." >/dev/null +untrusted_https=$(curl -fsS -H 'X-Forwarded-Proto: https' "$default_base/skillhub/install/skillhub.md") +printf '%s' "$untrusted_https" | grep -F "The primary registry for this guide is \`$default_base/skillhub\`." >/dev/null if printf '%s' "$default_guide" | grep -F '__SKILLHUB_PUBLIC_BASE_URL__' >/dev/null; then echo 'default Agent guide must not expose the runtime URL marker' >&2 exit 1 fi +for hostile_host in 'evil.example;echo_injected' 'evil.example$(id)' 'evil.example&whoami'; do + hostile_status=$(curl -sS -o "$tmp/hostile-response" -w '%{http_code}' -H "Host: $hostile_host" "$default_base/skillhub/install/skillhub.md") + if [ "$hostile_status" != 400 ]; then + echo "dynamic Agent guide must reject hostile Host, got $hostile_status for $hostile_host" >&2 + exit 1 + fi + if grep -F "$hostile_host" "$tmp/hostile-response" >/dev/null 2>&1; then + echo 'dynamic Agent guide must not echo a hostile Host' >&2 + exit 1 + fi +done docker rm -f "$name_default" >/dev/null 2>&1 || true +# A trusted proxy may supply one exact canonical scheme. Comma-separated or +# otherwise malformed values retain the direct request scheme. +trusted_html="$tmp/trusted-html" +mkdir -p "$trusted_html/assets" "$trusted_html/install" "$trusted_html/registry" +printf '%s\n' 'INDEX_HTML_MARKER' >"$trusted_html/index.html" +cp "$ROOT_DIR/web/src/docs/skill.md.template" "$trusted_html/registry/skill.md.template" +cp "$ROOT_DIR/web/runtime-config.js.template" "$trusted_html/runtime-config.js.template" +name_trusted="$name-trusted" +port_trusted=18083 +docker run -d --name "$name_trusted" \ + -p "$port_trusted:80" \ + -e SKILLHUB_API_UPSTREAM=http://127.0.0.1:9 \ + -e SKILLHUB_TRUST_FORWARDED_PROTO=true \ + -e SKILLHUB_WEB_BASE_PATH=/skillhub/ \ + -v "$trusted_html:/usr/share/nginx/html" \ + -v "$ROOT_DIR/web/nginx.conf.template:/etc/nginx/templates/default.conf.template:ro" \ + -v "$entrypoint_d/20-base-path.sh:/docker-entrypoint.d/20-base-path.sh:ro" \ + -v "$entrypoint_d/30-runtime-config.sh:/docker-entrypoint.d/30-runtime-config.sh:ro" \ + "$NGINX_IMAGE" >/dev/null +trusted_base="http://127.0.0.1:$port_trusted" +i=0 +until curl -fsS -o /dev/null "$trusted_base/nginx-health" 2>/dev/null; do + i=$((i + 1)) + if [ "$i" -ge 30 ]; then + echo 'nginx (trusted proxy) did not become ready' >&2 + docker logs "$name_trusted" >&2 || true + exit 1 + fi + sleep 1 +done +trusted_https=$(curl -fsS -H 'X-Forwarded-Proto: https' "$trusted_base/skillhub/install/skillhub.md") +printf '%s' "$trusted_https" | grep -F "The primary registry for this guide is \`https://127.0.0.1:$port_trusted/skillhub\`." >/dev/null +trusted_malformed=$(curl -fsS -H 'X-Forwarded-Proto: https,http' "$trusted_base/skillhub/install/skillhub.md") +printf '%s' "$trusted_malformed" | grep -F "The primary registry for this guide is \`$trusted_base/skillhub\`." >/dev/null +docker rm -f "$name_trusted" >/dev/null 2>&1 || true + # Fixed-base image served via the bundled deploy configs: assets are baked under # /fixed/, a baked-base marker is present, and SKILLHUB_WEB_BASE_PATH is passed as # an empty string (as compose.release.yml / k8s do). Routing must follow the baked diff --git a/web/docker-entrypoint.d/30-runtime-config.sh b/web/docker-entrypoint.d/30-runtime-config.sh index 952b286d..2ecaae35 100644 --- a/web/docker-entrypoint.d/30-runtime-config.sh +++ b/web/docker-entrypoint.d/30-runtime-config.sh @@ -25,10 +25,17 @@ envsubst '${SKILLHUB_WEB_API_BASE_URL} ${SKILLHUB_PUBLIC_BASE_URL} ${SKILLHUB_WE # one template so self-hosted deployments keep their own registry URL. mkdir -p /usr/share/nginx/html/install guide_public_base_url="$SKILLHUB_PUBLIC_BASE_URL" +guide_url_config="${SKILLHUB_NGINX_GUIDE_URL_CONFIG:-/etc/nginx/skillhub-guide-public-url.conf}" if [ -z "$guide_public_base_url" ]; then - # Nginx replaces this marker from the sanitized request scheme, Host, and - # configured base path. This keeps zero-config self-hosted installs usable. + # Nginx replaces this marker from the sanitized request scheme, a strictly + # allowlisted Host, and the configured base path. A Host outside this safe + # URL grammar must never reach copied shell commands in the guide. guide_public_base_url='__SKILLHUB_PUBLIC_BASE_URL__' + printf '%s\n' \ + 'if ($http_host !~ "^(?:[A-Za-z0-9.-]+|\\[[0-9A-Fa-f:.]+\\])(?::[0-9]{1,5})?$") { return 400; }' \ + > "$guide_url_config" +else + printf '%s\n' '# Explicit public URL: request Host is not used in the guide.' > "$guide_url_config" fi SKILLHUB_PUBLIC_BASE_URL="$guide_public_base_url" envsubst '${SKILLHUB_PUBLIC_BASE_URL}' \ < /usr/share/nginx/html/registry/skill.md.template \ diff --git a/web/nginx.conf.template b/web/nginx.conf.template index acc55bc4..0993c3d2 100644 --- a/web/nginx.conf.template +++ b/web/nginx.conf.template @@ -75,6 +75,7 @@ server { location = /registry/skill.md { default_type text/plain; + include /etc/nginx/skillhub-guide-public-url*.conf; sub_filter_types text/plain; sub_filter_once off; sub_filter '__SKILLHUB_PUBLIC_BASE_URL__' '$proxy_x_forwarded_proto://$http_host$skillhub_forwarded_prefix'; @@ -86,6 +87,7 @@ server { location = /install/skillhub.md { default_type text/plain; + include /etc/nginx/skillhub-guide-public-url*.conf; sub_filter_types text/plain; sub_filter_once off; sub_filter '__SKILLHUB_PUBLIC_BASE_URL__' '$proxy_x_forwarded_proto://$http_host$skillhub_forwarded_prefix';