diff --git a/.env.release.draft b/.env.release.draft index c8f0872b..417aab30 100644 --- a/.env.release.draft +++ b/.env.release.draft @@ -18,6 +18,9 @@ SKILLHUB_PUBLIC_BASE_URL=https://skillhub.example.com # Usually keep empty when web and api are served from the same domain. SKILLHUB_WEB_API_BASE_URL= SKILLHUB_API_UPSTREAM=http://server:8080 +# Enable only when a trusted TLS-terminating proxy replaces X-Forwarded-Proto +# and the web container cannot be reached directly. +SKILLHUB_TRUST_FORWARDED_PROTO=false # Keep database and redis local-only on the host unless you explicitly need remote access. POSTGRES_BIND_ADDRESS=127.0.0.1 diff --git a/.env.release.example b/.env.release.example index 2d30c7bc..d038d6e1 100644 --- a/.env.release.example +++ b/.env.release.example @@ -15,6 +15,9 @@ SKILLHUB_PUBLIC_BASE_URL=http://localhost # Frontend usually keeps this empty and proxies to the backend through nginx. SKILLHUB_WEB_API_BASE_URL= SKILLHUB_API_UPSTREAM=http://server:8080 +# Keep false for direct exposure. Enable only behind a trusted proxy that replaces +# X-Forwarded-Proto and blocks direct access to the web container. +SKILLHUB_TRUST_FORWARDED_PROTO=false POSTGRES_BIND_ADDRESS=127.0.0.1 POSTGRES_PORT=5432 diff --git a/.github/workflows/pr-scripts.yml b/.github/workflows/pr-scripts.yml index e521eb9e..082ce102 100644 --- a/.github/workflows/pr-scripts.yml +++ b/.github/workflows/pr-scripts.yml @@ -8,6 +8,8 @@ on: - '.env.release.draft' - 'compose.release.yml' - 'Makefile' + - 'web/Dockerfile' + - 'web/nginx.conf.template' - '.github/workflows/pr-cli.yml' - '.github/workflows/pr-e2e.yml' - '.github/workflows/pr-tests.yml' @@ -33,5 +35,6 @@ jobs: - run: bash scripts/tests/publish-cli-test.sh - run: bash scripts/tests/runtime-secret-test.sh - run: bash scripts/tests/validate-release-config-test.sh + - run: bash scripts/tests/nginx-forwarded-proto-test.sh - run: bash scripts/tests/dev-web-host-test.sh - run: bash scripts/tests/workflow-security-test.sh diff --git a/compose.release.yml b/compose.release.yml index 5ed7086e..69c07496 100644 --- a/compose.release.yml +++ b/compose.release.yml @@ -116,6 +116,7 @@ services: - "${WEB_PORT:-80}:80" environment: SKILLHUB_API_UPSTREAM: ${SKILLHUB_API_UPSTREAM:-http://server:8080} + SKILLHUB_TRUST_FORWARDED_PROTO: ${SKILLHUB_TRUST_FORWARDED_PROTO:-false} SKILLHUB_WEB_API_BASE_URL: ${SKILLHUB_WEB_API_BASE_URL:-} SKILLHUB_PUBLIC_BASE_URL: ${SKILLHUB_PUBLIC_BASE_URL:-} SKILLHUB_WEB_AUTH_DIRECT_ENABLED: ${SKILLHUB_WEB_AUTH_DIRECT_ENABLED:-false} diff --git a/docs/09-deployment.md b/docs/09-deployment.md index fe631d80..48c5d9d0 100644 --- a/docs/09-deployment.md +++ b/docs/09-deployment.md @@ -194,6 +194,9 @@ docker compose --env-file .env.release -f compose.release.yml up -d - 推荐将敏感变量放入 CI/CD Secret 或主机上的受控 `.env.release` - 外部对象存储通过 `SKILLHUB_STORAGE_S3_*` 注入 - 前端反代和运行时 API 地址通过 `SKILLHUB_API_UPSTREAM` / `SKILLHUB_WEB_API_BASE_URL` 注入 +- `SKILLHUB_TRUST_FORWARDED_PROTO` 默认保持 `false`。只有 Web 容器仅能经由可信 + TLS 终止代理访问,且该代理会覆盖客户端传入的 `X-Forwarded-Proto` 时才设为 + `true`;否则客户端可伪造协议并影响 OAuth 回调、重定向和安全 Cookie 判断 - 如果要开放真实登录,再补充 `OAUTH2_GITHUB_CLIENT_ID` / `OAUTH2_GITHUB_CLIENT_SECRET` - 如果要启用密码重置验证码邮件,参见:`docs/19-smtp-password-reset-email-setup.md` diff --git a/scripts/tests/nginx-forwarded-proto-test.sh b/scripts/tests/nginx-forwarded-proto-test.sh new file mode 100755 index 00000000..01be85c3 --- /dev/null +++ b/scripts/tests/nginx-forwarded-proto-test.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +TEMPLATE="$REPO_ROOT/web/nginx.conf.template" +NGINX_IMAGE="${NGINX_TEST_IMAGE:-nginx:alpine}" +TEST_ID="skillhub-nginx-forwarded-proto-$$" +NETWORK="${TEST_ID}-network" +BACKEND="${TEST_ID}-backend" +DEFAULT_PROXY="${TEST_ID}-default" +TRUSTED_PROXY="${TEST_ID}-trusted" +TMP_DIR="$(mktemp -d)" +CONTAINERS=() + +cleanup() { + if ((${#CONTAINERS[@]} > 0)); then + docker rm -f "${CONTAINERS[@]}" >/dev/null 2>&1 || true + fi + docker network rm "$NETWORK" >/dev/null 2>&1 || true + rm -rf "$TMP_DIR" +} +trap cleanup EXIT + +fail() { + echo "FAIL: $*" >&2 + exit 1 +} + +wait_for_nginx() { + local container="$1" + local attempt + for attempt in {1..30}; do + if docker exec "$container" wget -qO- http://127.0.0.1/nginx-health >/dev/null 2>&1; then + return 0 + fi + sleep 0.2 + done + docker logs "$container" >&2 || true + fail "$container did not become healthy" +} + +start_proxy() { + local container="$1" + local trust_forwarded_proto="$2" + docker run --detach \ + --name "$container" \ + --network "$NETWORK" \ + --env "SKILLHUB_API_UPSTREAM=http://$BACKEND:8080" \ + --env "SKILLHUB_TRUST_FORWARDED_PROTO=$trust_forwarded_proto" \ + --volume "$TEMPLATE:/etc/nginx/templates/default.conf.template:ro" \ + "$NGINX_IMAGE" >/dev/null + CONTAINERS+=("$container") + wait_for_nginx "$container" +} + +assert_proto() { + local container="$1" + local expected="$2" + local header="${3:-}" + local path="${4:-/api/proto}" + local actual + if [[ -n "$header" ]]; then + actual="$(docker exec "$container" wget -qO- \ + --header="X-Forwarded-Proto: $header" \ + "http://127.0.0.1$path")" + else + actual="$(docker exec "$container" wget -qO- "http://127.0.0.1$path")" + fi + [[ "$actual" == "$expected" ]] \ + || fail "$container forwarded proto '$actual', expected '$expected' for $path with header '${header:-}'" +} + +cat >"$TMP_DIR/backend.conf" <<'EOF' +server { + listen 8080; + location / { + default_type text/plain; + return 200 $http_x_forwarded_proto; + } +} +EOF + +docker network create "$NETWORK" >/dev/null +docker run --detach \ + --name "$BACKEND" \ + --network "$NETWORK" \ + --volume "$TMP_DIR/backend.conf:/etc/nginx/conf.d/default.conf:ro" \ + "$NGINX_IMAGE" >/dev/null +CONTAINERS+=("$BACKEND") + +start_proxy "$DEFAULT_PROXY" false +start_proxy "$TRUSTED_PROXY" true + +for path in /api/proto /oauth2/proto /login/oauth2/proto /.well-known/proto; do + assert_proto "$DEFAULT_PROXY" http https "$path" + assert_proto "$TRUSTED_PROXY" https https "$path" +done +assert_proto "$TRUSTED_PROXY" http +assert_proto "$TRUSTED_PROXY" http "https,http" + +echo "nginx-forwarded-proto-test passed" diff --git a/scripts/tests/validate-release-config-test.sh b/scripts/tests/validate-release-config-test.sh index fd76ddfc..d94ed62c 100755 --- a/scripts/tests/validate-release-config-test.sh +++ b/scripts/tests/validate-release-config-test.sh @@ -36,6 +36,7 @@ POSTGRES_USER=skillhub POSTGRES_PASSWORD=strong-postgres-password SESSION_COOKIE_SECURE=true BOOTSTRAP_ADMIN_ENABLED=false +SKILLHUB_TRUST_FORWARDED_PROTO=false SKILLHUB_STORAGE_PROVIDER=s3 SKILLHUB_STORAGE_S3_ENDPOINT=https://storage.example.com SKILLHUB_STORAGE_S3_BUCKET=skillhub @@ -80,6 +81,11 @@ short_env="$tmp/short.env" write_env "$short_env" "too-short" expect_fail "$short_env" "SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET must be at least 32 characters" +invalid_forwarded_proto_env="$tmp/invalid-forwarded-proto.env" +write_env "$invalid_forwarded_proto_env" "release-download-secret-32-bytes-minimum" +printf '%s\n' "SKILLHUB_TRUST_FORWARDED_PROTO=yes" >>"$invalid_forwarded_proto_env" +expect_fail "$invalid_forwarded_proto_env" "SKILLHUB_TRUST_FORWARDED_PROTO must be true or false" + draft_env="$tmp/draft.env" while IFS= read -r line || [[ -n "$line" ]]; do case "$line" in diff --git a/scripts/tests/workflow-security-test.sh b/scripts/tests/workflow-security-test.sh index 9a1688ef..ec1f70ce 100755 --- a/scripts/tests/workflow-security-test.sh +++ b/scripts/tests/workflow-security-test.sh @@ -64,8 +64,14 @@ grep -Fq '.env.release.draft' "$PR_SCRIPTS_WORKFLOW" \ || fail "pr-scripts must run when release env draft changes" grep -Fq 'compose.release.yml' "$PR_SCRIPTS_WORKFLOW" \ || fail "pr-scripts must run when release compose changes" +grep -Fq 'web/Dockerfile' "$PR_SCRIPTS_WORKFLOW" \ + || fail "pr-scripts must run when the web image changes" +grep -Fq 'web/nginx.conf.template' "$PR_SCRIPTS_WORKFLOW" \ + || fail "pr-scripts must run when the nginx template changes" grep -Fq 'bash scripts/tests/validate-release-config-test.sh' "$PR_SCRIPTS_WORKFLOW" \ || fail "pr-scripts must run validate-release-config-test" +grep -Fq 'bash scripts/tests/nginx-forwarded-proto-test.sh' "$PR_SCRIPTS_WORKFLOW" \ + || fail "pr-scripts must run nginx-forwarded-proto-test" grep -Fq 'bash scripts/tests/runtime-secret-test.sh' "$PR_SCRIPTS_WORKFLOW" \ || fail "pr-scripts must run runtime-secret-test" grep -Fq 'bash scripts/tests/dev-web-host-test.sh' "$PR_SCRIPTS_WORKFLOW" \ diff --git a/scripts/validate-release-config.sh b/scripts/validate-release-config.sh index 03e42bd8..27e9d042 100755 --- a/scripts/validate-release-config.sh +++ b/scripts/validate-release-config.sh @@ -151,6 +151,7 @@ reject_patterns SPRING_MAIL_PASSWORD "TODO_*" "todo_*" "replace*" validate_boolean SESSION_COOKIE_SECURE validate_boolean BOOTSTRAP_ADMIN_ENABLED +validate_boolean SKILLHUB_TRUST_FORWARDED_PROTO validate_boolean SKILLHUB_STORAGE_S3_FORCE_PATH_STYLE validate_boolean SKILLHUB_STORAGE_S3_AUTO_CREATE_BUCKET diff --git a/web/Dockerfile b/web/Dockerfile index e301f7c8..2ed67ae0 100644 --- a/web/Dockerfile +++ b/web/Dockerfile @@ -7,6 +7,7 @@ COPY . . RUN pnpm build FROM nginx:alpine +ENV SKILLHUB_TRUST_FORWARDED_PROTO=false COPY --from=build /app/dist /usr/share/nginx/html COPY --from=build /app/src/docs/skill.md.template /usr/share/nginx/html/registry/skill.md.template COPY nginx.conf.template /etc/nginx/templates/default.conf.template diff --git a/web/nginx.conf.template b/web/nginx.conf.template index be2a51a2..25db2869 100644 --- a/web/nginx.conf.template +++ b/web/nginx.conf.template @@ -10,9 +10,15 @@ server { gzip_types text/plain text/css application/json application/javascript text/xml; gzip_min_length 1000; + # Ignore client-supplied forwarded proto by default. Operators may explicitly trust a + # sanitizing upstream proxy; only canonical http/https values are then accepted. set $proxy_x_forwarded_proto $scheme; - if ($http_x_forwarded_proto) { - set $proxy_x_forwarded_proto $http_x_forwarded_proto; + set $forwarded_proto_source "${SKILLHUB_TRUST_FORWARDED_PROTO}:$http_x_forwarded_proto"; + if ($forwarded_proto_source ~* "^true:https$") { + set $proxy_x_forwarded_proto https; + } + if ($forwarded_proto_source ~* "^true:http$") { + set $proxy_x_forwarded_proto http; } location / { @@ -31,6 +37,7 @@ server { proxy_pass ${SKILLHUB_API_UPSTREAM}; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto; } @@ -38,12 +45,15 @@ server { proxy_pass ${SKILLHUB_API_UPSTREAM}; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto; } location /.well-known/ { proxy_pass ${SKILLHUB_API_UPSTREAM}; proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto; }