fix(nginx): trust forwarded proto only when configured

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>

.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

.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

.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

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}

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`
 

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:-<none>}'"
+}
+
+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"

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

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" \

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
 

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

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;
     }