diff --git a/.env.release.example b/.env.release.example index 049d887f..20c3d438 100644 --- a/.env.release.example +++ b/.env.release.example @@ -19,6 +19,13 @@ SKILLHUB_API_UPSTREAM=http://server:8080 # X-Forwarded-Proto and blocks direct access to the web container. SKILLHUB_TRUST_FORWARDED_PROTO=false +# Sub-path deployment example. Keep all three public/browser values aligned: +# SKILLHUB_PUBLIC_BASE_URL=https://example.com/skillhub +# SKILLHUB_WEB_API_BASE_URL=/skillhub +# SKILLHUB_WEB_BASE_PATH=/skillhub/ +# Leave empty so a fixed-base image keeps its baked base; set to a sub-path to override. +SKILLHUB_WEB_BASE_PATH= + POSTGRES_BIND_ADDRESS=127.0.0.1 POSTGRES_PORT=5432 POSTGRES_DB=skillhub diff --git a/.github/workflows/pr-scripts.yml b/.github/workflows/pr-scripts.yml index d49466e7..937dc05d 100644 --- a/.github/workflows/pr-scripts.yml +++ b/.github/workflows/pr-scripts.yml @@ -10,6 +10,7 @@ on: - 'Makefile' - 'web/Dockerfile' - 'web/nginx.conf.template' + - 'web/docker-entrypoint.d/**' - '.github/workflows/pr-cli.yml' - '.github/workflows/pr-e2e.yml' - '.github/workflows/pr-helm-chart.yml' @@ -39,5 +40,7 @@ jobs: - run: bash scripts/tests/validate-release-config-test.sh - run: bash scripts/tests/nginx-forwarded-proto-test.sh - run: bash scripts/tests/smoke-test-admin-mode-test.sh + - run: bash scripts/tests/web-base-path-routing-test.sh + - run: bash scripts/tests/web-base-path-nginx-smoke-test.sh - run: bash scripts/tests/dev-web-host-test.sh - run: bash scripts/tests/workflow-security-test.sh diff --git a/charts/skillhub/templates/configmap.yaml b/charts/skillhub/templates/configmap.yaml index 78e9e016..e82a6376 100644 --- a/charts/skillhub/templates/configmap.yaml +++ b/charts/skillhub/templates/configmap.yaml @@ -54,4 +54,8 @@ data: device-auth-verification-uri: {{ $deviceAuthVerificationUri | quote }} auth-direct-enabled: {{ .Values.auth.direct.enabled | quote }} auth-direct-provider: {{ .Values.auth.direct.provider | quote }} + + # Sub-path deployment (empty keeps a fixed-base image's baked base; set e.g. /portal/) + web-base-path: {{ .Values.web.basePath | default "" | quote }} + web-api-base-url: {{ .Values.web.apiBaseUrl | default "" | quote }} builtin-skills-enabled: {{ .Values.builtinSkills.enabled | quote }} diff --git a/charts/skillhub/templates/validate.yaml b/charts/skillhub/templates/validate.yaml index db34c848..667a59e9 100644 --- a/charts/skillhub/templates/validate.yaml +++ b/charts/skillhub/templates/validate.yaml @@ -24,6 +24,37 @@ {{- end -}} {{- end -}} +{{- if .Values.publicBaseUrl -}} +{{- if not (regexMatch $absoluteHttpUrlPattern .Values.publicBaseUrl) -}} +{{- fail (printf "publicBaseUrl must be an absolute http(s) URL with a host (e.g. https://skills.example.com): %s" .Values.publicBaseUrl) -}} +{{- end -}} +{{- if regexMatch "[?#]" .Values.publicBaseUrl -}} +{{- fail (printf "publicBaseUrl must not contain a query ('?') or fragment ('#'); it is concatenated with paths like /cli/auth and /.well-known/clawhub.json: %s" .Values.publicBaseUrl) -}} +{{- end -}} +{{- end -}} + +{{- $webBasePath := .Values.web.basePath | default "" -}} +{{- if and (ne $webBasePath "") (ne $webBasePath "/") -}} +{{- if not (regexMatch "^(/[A-Za-z0-9_~-][A-Za-z0-9._~-]*)+/$" $webBasePath) -}} +{{- fail (printf "web.basePath must be '/' or a normalized sub-path that starts and ends with '/' and has no '.'/'..' or empty segments (matches the runtime and release-config checks): %s" $webBasePath) -}} +{{- end -}} +{{- $firstSegment := index (splitList "/" $webBasePath) 1 -}} +{{- if has $firstSegment (list "api" "oauth2" "login" "assets" "registry" "nginx-health" ".well-known" "runtime-config.js") -}} +{{- fail (printf "web.basePath must not start with a segment reserved by the SkillHub server (%s); it would shadow the server's own Nginx location: %s" $firstSegment $webBasePath) -}} +{{- end -}} +{{- $suffix := trimSuffix "/" $webBasePath -}} +{{- if .Values.publicBaseUrl -}} +{{- $publicPath := trimSuffix "/" (regexReplaceAll "^[a-zA-Z][a-zA-Z0-9+.-]*://[^/]+" .Values.publicBaseUrl "") -}} +{{- if ne $publicPath $suffix -}} +{{- fail (printf "publicBaseUrl path (%s) must equal web.basePath without its trailing slash (%s) so CLI, install, and Quick Start URLs keep the prefix" $publicPath $suffix) -}} +{{- end -}} +{{- end -}} +{{- $apiBase := .Values.web.apiBaseUrl | default "" -}} +{{- if and (ne $apiBase "") (not (regexMatch "^https?://" $apiBase)) (ne $apiBase $suffix) -}} +{{- fail (printf "web.apiBaseUrl (%s) must equal web.basePath without its trailing slash (%s) for same-origin sub-path routing, or be an absolute URL for a separate API host" $apiBase $suffix) -}} +{{- end -}} +{{- end -}} + {{- range $name := list "server" "web" "scanner" -}} {{- $component := index $.Values $name -}} {{- $enabled := true -}} diff --git a/charts/skillhub/templates/web-deployment.yaml b/charts/skillhub/templates/web-deployment.yaml index bdbb3e75..7ad2e403 100644 --- a/charts/skillhub/templates/web-deployment.yaml +++ b/charts/skillhub/templates/web-deployment.yaml @@ -48,6 +48,16 @@ spec: configMapKeyRef: name: {{ include "skillhub.fullname" . }}-config key: auth-direct-provider + - name: SKILLHUB_WEB_BASE_PATH + valueFrom: + configMapKeyRef: + name: {{ include "skillhub.fullname" . }}-config + key: web-base-path + - name: SKILLHUB_WEB_API_BASE_URL + valueFrom: + configMapKeyRef: + name: {{ include "skillhub.fullname" . }}-config + key: web-api-base-url {{- with .Values.web.extraEnv }} {{- toYaml . | nindent 12 }} {{- end }} diff --git a/charts/skillhub/tests/configuration-contracts.sh b/charts/skillhub/tests/configuration-contracts.sh index 94df9733..20e98d3f 100755 --- a/charts/skillhub/tests/configuration-contracts.sh +++ b/charts/skillhub/tests/configuration-contracts.sh @@ -318,4 +318,47 @@ if helm template missing-credentials "$CHART_DIR" >"$TMP_DIR/missing-credentials fail "default rendering without stable credentials should have been rejected" fi +# Sub-path deployment: base path and API prefix must flow from values into the +# config map and be injected into the web deployment. +grep -Fq 'web-base-path: ""' "$TMP_DIR/default.yaml" \ + || fail "default config map web-base-path must be empty so a fixed-base image is honored" + +render subpath "$CHART_DIR" \ + --set web.basePath=/portal/ \ + --set web.apiBaseUrl=/portal >"$TMP_DIR/subpath.yaml" +grep -Fq 'web-base-path: "/portal/"' "$TMP_DIR/subpath.yaml" \ + || fail "config map must expose the configured web base path" +grep -Fq 'web-api-base-url: "/portal"' "$TMP_DIR/subpath.yaml" \ + || fail "config map must expose the configured web API base url" +grep -Fq 'name: SKILLHUB_WEB_BASE_PATH' "$TMP_DIR/subpath.yaml" \ + || fail "web deployment must set SKILLHUB_WEB_BASE_PATH" +grep -Fq 'key: web-base-path' "$TMP_DIR/subpath.yaml" \ + || fail "web deployment must source SKILLHUB_WEB_BASE_PATH from the config map" +grep -Fq 'key: web-api-base-url' "$TMP_DIR/subpath.yaml" \ + || fail "web deployment must source SKILLHUB_WEB_API_BASE_URL from the config map" + +# A sub-path base must be consistent with publicBaseUrl and be a normalized path. +assert_rejected subpath-public-mismatch \ + --set web.basePath=/portal/ \ + --set-string publicBaseUrl=https://skills.example.com +assert_rejected subpath-dot-segment --set web.basePath=/foo/../bar/ +assert_rejected subpath-missing-trailing --set-string web.basePath=/portal +assert_rejected subpath-api-base-mismatch \ + --set web.basePath=/portal/ \ + --set-string web.apiBaseUrl=/other \ + --set-string publicBaseUrl=https://skills.example.com/portal +# A base path whose first segment is reserved by the server would shadow the +# server's own Nginx location and break the app. +assert_rejected subpath-reserved-api --set-string web.basePath=/api/ +assert_rejected subpath-reserved-assets --set-string web.basePath=/assets/ +assert_rejected subpath-reserved-well-known --set-string web.basePath=/.well-known/ +assert_rejected subpath-reserved-nested --set-string web.basePath=/api/nested/ + +# publicBaseUrl is concatenated with paths (/cli/auth, /.well-known/clawhub.json), +# so a query or fragment corrupts the generated URLs. Reject it independently of +# web.basePath (these cases use the default root deployment). +assert_rejected public-base-url-query --set-string publicBaseUrl=https://skills.example.com/skillhub?ref=1 +assert_rejected public-base-url-fragment --set-string publicBaseUrl=https://skills.example.com#frag +assert_rejected public-base-url-no-host --set-string publicBaseUrl=https:// + echo "Helm configuration contract tests passed" diff --git a/charts/skillhub/values.schema.json b/charts/skillhub/values.schema.json index 19be6440..30c62503 100644 --- a/charts/skillhub/values.schema.json +++ b/charts/skillhub/values.schema.json @@ -16,7 +16,7 @@ }, "nameOverride": { "$ref": "#/definitions/optionalDnsLabel" }, "fullnameOverride": { "$ref": "#/definitions/optionalDnsLabel" }, - "publicBaseUrl": { "type": "string" }, + "publicBaseUrl": { "type": "string", "pattern": "^$|^https?://[^/?#]+([/][^?#]*)?$" }, "deviceAuthVerificationUri": { "type": "string" }, "auth": { "type": "object", @@ -439,6 +439,8 @@ "replicaCount": { "type": "integer", "minimum": 1 }, "image": { "$ref": "#/definitions/image" }, "service": { "$ref": "#/definitions/service" }, + "basePath": { "type": "string", "pattern": "^$|^/$|^(/[A-Za-z0-9_~-][A-Za-z0-9._~-]*)+/$" }, + "apiBaseUrl": { "type": "string" }, "resources": { "$ref": "#/definitions/resources" }, "extraEnv": { "type": "array", "items": { "type": "object" } }, "podAnnotations": { "$ref": "#/definitions/stringMap" }, diff --git a/charts/skillhub/values.yaml b/charts/skillhub/values.yaml index 059ff73c..d99b47f1 100644 --- a/charts/skillhub/values.yaml +++ b/charts/skillhub/values.yaml @@ -369,6 +369,12 @@ web: registry: "" tag: "" + # Sub-path deployment. Leave empty to keep a fixed-base image's baked base (or + # root for a placeholder-base image). Set to a sub-path such as "/portal/" to + # serve the app under a prefix; keep apiBaseUrl aligned (e.g. "/portal"). + basePath: "" + apiBaseUrl: "" + service: enabled: true type: ClusterIP diff --git a/compose.release.yml b/compose.release.yml index 75f018ee..8428d6e1 100644 --- a/compose.release.yml +++ b/compose.release.yml @@ -142,6 +142,8 @@ services: 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:-} + # Leave empty so a fixed-base image keeps its baked base; set to e.g. /skillhub/ to override. + SKILLHUB_WEB_BASE_PATH: ${SKILLHUB_WEB_BASE_PATH:-} SKILLHUB_PUBLIC_BASE_URL: ${SKILLHUB_PUBLIC_BASE_URL:-} SKILLHUB_WEB_AUTH_DIRECT_ENABLED: ${SKILLHUB_WEB_AUTH_DIRECT_ENABLED:-false} SKILLHUB_WEB_AUTH_DIRECT_PROVIDER: ${SKILLHUB_WEB_AUTH_DIRECT_PROVIDER:-} diff --git a/deploy/k8s/base/frontend-deployment.yaml b/deploy/k8s/base/frontend-deployment.yaml index 35501c24..2a03ec9e 100644 --- a/deploy/k8s/base/frontend-deployment.yaml +++ b/deploy/k8s/base/frontend-deployment.yaml @@ -21,6 +21,11 @@ spec: env: - name: SKILLHUB_API_UPSTREAM value: http://skillhub-server:8080 + # Leave empty so a fixed-base image keeps its baked base; set to e.g. /skillhub/ to override. + - name: SKILLHUB_WEB_BASE_PATH + value: "" + - name: SKILLHUB_WEB_API_BASE_URL + value: "" ports: - containerPort: 80 name: http diff --git a/docs/09-deployment.md b/docs/09-deployment.md index 6a6a5276..3b67a771 100644 --- a/docs/09-deployment.md +++ b/docs/09-deployment.md @@ -246,6 +246,11 @@ Sentinel 配置优先于 Cluster 和单机 `host`/`port`。在 Kubernetes 等 Se - `SKILLHUB_TRUST_FORWARDED_PROTO` 默认保持 `false`。只有 Web 容器仅能经由可信 TLS 终止代理访问,且该代理会覆盖客户端传入的 `X-Forwarded-Proto` 时才设为 `true`;否则客户端可伪造协议并影响 OAuth 回调、重定向和安全 Cookie 判断 +- 如果通过网关部署在 `/skillhub/` 等子路径,需同时配置: + - `SKILLHUB_WEB_BASE_PATH=/skillhub/` + - `SKILLHUB_WEB_API_BASE_URL=/skillhub` + - `SKILLHUB_PUBLIC_BASE_URL=https://example.com/skillhub` + 网关可以在转发到 Web 容器前将该前缀重写掉,但公网 URL 仍必须保留前缀,确保 OAuth、CLI 和 registry 链接正确。 - 如果要开放真实登录,再补充 `OAUTH2_GITHUB_CLIENT_ID` / `OAUTH2_GITHUB_CLIENT_SECRET` - 如果要启用密码重置验证码邮件,参见:`docs/19-smtp-password-reset-email-setup.md` @@ -298,7 +303,7 @@ override 或部署平台环境变量把上述 `SPRING_SECURITY_*` 变量注入 ` - 配置公网 HTTPS 入口,确保最终访问域名已经确定 - 打开 `80` / `443`,避免直接暴露 `5432` / `6379` 2. 填写 `.env.release` - - `SKILLHUB_PUBLIC_BASE_URL` 填最终 HTTPS 域名,且不要带尾部 `/` + - `SKILLHUB_PUBLIC_BASE_URL` 填最终 HTTPS 域名,且不要带尾部 `/`;子路径部署时必须包含外部路径前缀 - `SKILLHUB_STORAGE_PROVIDER=s3` - 按云厂商 OSS / S3 兼容参数填写 `SKILLHUB_STORAGE_S3_*` - 设置非默认的 `POSTGRES_PASSWORD` diff --git a/scripts/tests/validate-release-config-test.sh b/scripts/tests/validate-release-config-test.sh index af876a9c..1ec54043 100755 --- a/scripts/tests/validate-release-config-test.sh +++ b/scripts/tests/validate-release-config-test.sh @@ -75,6 +75,98 @@ write_env "$disabled_builtin_skills_env" "release-download-secret-32-bytes-minim printf '%s\n' "SKILLHUB_BUILTIN_SKILLS_ENABLED=false" >>"$disabled_builtin_skills_env" "$SCRIPT" "$disabled_builtin_skills_env" >/dev/null +relative_api_base_env="$tmp/relative-api-base.env" +write_env "$relative_api_base_env" "release-download-secret-32-bytes-minimum" +printf 'SKILLHUB_WEB_API_BASE_URL=/skillhub\n' >>"$relative_api_base_env" +"$SCRIPT" "$relative_api_base_env" >/dev/null + +sub_path_env="$tmp/sub-path.env" +write_env "$sub_path_env" "release-download-secret-32-bytes-minimum" +printf '%s\n' \ + 'SKILLHUB_WEB_BASE_PATH=/skillhub/' \ + 'SKILLHUB_WEB_API_BASE_URL=/skillhub' \ + 'SKILLHUB_PUBLIC_BASE_URL=https://skillhub.example.com/skillhub' \ + >"$tmp/sub-path-overrides" +cat "$tmp/sub-path-overrides" >>"$sub_path_env" +"$SCRIPT" "$sub_path_env" >/dev/null + +missing_public_sub_path_env="$tmp/missing-public-sub-path.env" +write_env "$missing_public_sub_path_env" "release-download-secret-32-bytes-minimum" +printf '%s\n' \ + 'SKILLHUB_WEB_BASE_PATH=/skillhub/' \ + 'SKILLHUB_WEB_API_BASE_URL=/skillhub' \ + >>"$missing_public_sub_path_env" +expect_fail "$missing_public_sub_path_env" "SKILLHUB_PUBLIC_BASE_URL path" + +# A public URL that merely ends with the base path but serves it under a different path +# (/other/skillhub) must be rejected: suffix match is not enough, the path must be exact. +wrong_public_path_env="$tmp/wrong-public-path.env" +write_env "$wrong_public_path_env" "release-download-secret-32-bytes-minimum" +printf '%s\n' \ + 'SKILLHUB_WEB_BASE_PATH=/skillhub/' \ + 'SKILLHUB_WEB_API_BASE_URL=/skillhub' \ + 'SKILLHUB_PUBLIC_BASE_URL=https://skillhub.example.com/other/skillhub' \ + >>"$wrong_public_path_env" +expect_fail "$wrong_public_path_env" "must equal SKILLHUB_WEB_BASE_PATH without its trailing slash" + +# A query or fragment in SKILLHUB_PUBLIC_BASE_URL corrupts concatenated URLs +# (e.g. ${SKILLHUB_PUBLIC_BASE_URL}/cli/auth). Reject it independently of the +# base path, so even a root deployment (no SKILLHUB_WEB_BASE_PATH) is covered. +public_query_env="$tmp/public-query.env" +write_env "$public_query_env" "release-download-secret-32-bytes-minimum" +printf '%s\n' 'SKILLHUB_PUBLIC_BASE_URL=https://skillhub.example.com/skillhub?ref=1' \ + >>"$public_query_env" +expect_fail "$public_query_env" "must not contain a query" + +public_fragment_env="$tmp/public-fragment.env" +write_env "$public_fragment_env" "release-download-secret-32-bytes-minimum" +printf '%s\n' 'SKILLHUB_PUBLIC_BASE_URL=https://skillhub.example.com#frag' \ + >>"$public_fragment_env" +expect_fail "$public_fragment_env" "must not contain a query" + +# A scheme with no host (https://) passes a naive prefix check but concatenates +# into an invalid URL like https:///cli/auth. Must be rejected. +public_no_host_env="$tmp/public-no-host.env" +write_env "$public_no_host_env" "release-download-secret-32-bytes-minimum" +printf '%s\n' 'SKILLHUB_PUBLIC_BASE_URL=https://' >>"$public_no_host_env" +expect_fail "$public_no_host_env" "must include a host" + +# Base path format must be rejected here, matching the runtime entrypoint check, +# rather than passing config validation and only failing at container start. +dot_segment_base_env="$tmp/dot-segment-base.env" +write_env "$dot_segment_base_env" "release-download-secret-32-bytes-minimum" +printf 'SKILLHUB_WEB_BASE_PATH=/foo/../bar/\n' >>"$dot_segment_base_env" +expect_fail "$dot_segment_base_env" "must not contain '.' or '..' path segments" + +double_slash_base_env="$tmp/double-slash-base.env" +write_env "$double_slash_base_env" "release-download-secret-32-bytes-minimum" +printf 'SKILLHUB_WEB_BASE_PATH=/foo//bar/\n' >>"$double_slash_base_env" +expect_fail "$double_slash_base_env" "SKILLHUB_WEB_BASE_PATH contains unsupported characters" + +missing_trailing_base_env="$tmp/missing-trailing-base.env" +write_env "$missing_trailing_base_env" "release-download-secret-32-bytes-minimum" +printf 'SKILLHUB_WEB_BASE_PATH=/skillhub\n' >>"$missing_trailing_base_env" +expect_fail "$missing_trailing_base_env" "must be '/' or start and end with '/'" + +# A base path whose first segment collides with a server Nginx location (/api/, +# /oauth2/, ...) must be rejected: it would shadow the real route and break the app. +for reserved in /api/ /oauth2/ /login/ /assets/ /registry/ /nginx-health/ /.well-known/ /runtime-config.js/ /api/nested/; do + reserved_base_env="$tmp/reserved-base.env" + write_env "$reserved_base_env" "release-download-secret-32-bytes-minimum" + printf 'SKILLHUB_WEB_BASE_PATH=%s\n' "$reserved" >>"$reserved_base_env" + expect_fail "$reserved_base_env" "reserved by the SkillHub server" +done + +# A same-origin API base that disagrees with the web base path routes to the wrong prefix. +mismatch_api_base_env="$tmp/mismatch-api-base.env" +write_env "$mismatch_api_base_env" "release-download-secret-32-bytes-minimum" +printf '%s\n' \ + 'SKILLHUB_WEB_BASE_PATH=/skillhub/' \ + 'SKILLHUB_WEB_API_BASE_URL=/other' \ + 'SKILLHUB_PUBLIC_BASE_URL=https://skillhub.example.com/skillhub' \ + >>"$mismatch_api_base_env" +expect_fail "$mismatch_api_base_env" "must equal SKILLHUB_WEB_BASE_PATH without its trailing slash" + missing_env="$tmp/missing.env" write_env "$missing_env" "" no expect_fail "$missing_env" "SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET is required" diff --git a/scripts/tests/web-base-path-nginx-smoke-test.sh b/scripts/tests/web-base-path-nginx-smoke-test.sh new file mode 100755 index 00000000..446f3fff --- /dev/null +++ b/scripts/tests/web-base-path-nginx-smoke-test.sh @@ -0,0 +1,140 @@ +#!/bin/sh +set -eu + +# Real container smoke test: runs the actual nginx:alpine entrypoint with the +# repo's nginx template + 20-base-path.sh, then verifies over HTTP that a +# sub-path deployment serves real assets (not the SPA fallback) and redirects +# the bare prefix. This catches routing regressions that a text-only check +# cannot (e.g. assets falling through to index.html). + +if ! command -v docker >/dev/null 2>&1; then + printf '%s\n' 'web-base-path-nginx-smoke-test skipped (docker unavailable)' + exit 0 +fi + +ROOT_DIR=$(CDPATH= cd -- "$(dirname -- "$0")/../.." && pwd) +NGINX_IMAGE="${NGINX_SMOKE_IMAGE:-nginx:alpine}" +name="skillhub-base-path-smoke-$$" +port=18080 + +tmp=$(mktemp -d) +cleanup() { + docker rm -f "$name" "$name-fixed" >/dev/null 2>&1 || true + rm -rf "$tmp" +} +trap cleanup EXIT + +html="$tmp/html" +mkdir -p "$html/assets" +printf '%s\n' 'INDEX_HTML_MARKER' >"$html/index.html" +printf '%s\n' 'APP_JS_MARKER' >"$html/assets/app.js" + +# The image build chmods the entrypoint scripts; here we mount a copy and make it +# executable, since the nginx entrypoint silently ignores non-executable *.sh. +entrypoint_d="$tmp/entrypoint.d" +mkdir -p "$entrypoint_d" +cp "$ROOT_DIR/web/docker-entrypoint.d/20-base-path.sh" "$entrypoint_d/20-base-path.sh" +chmod +x "$entrypoint_d/20-base-path.sh" + +if ! docker run -d --name "$name" \ + -p "$port:80" \ + -e SKILLHUB_API_UPSTREAM=http://127.0.0.1:9 \ + -e SKILLHUB_TRUST_FORWARDED_PROTO=false \ + -e SKILLHUB_WEB_BASE_PATH=/skillhub/ \ + -v "$html:/usr/share/nginx/html:ro" \ + -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" \ + "$NGINX_IMAGE" >/dev/null 2>&1; then + printf '%s\n' 'web-base-path-nginx-smoke-test skipped (docker run failed, e.g. no image/network)' + exit 0 +fi + +base="http://127.0.0.1:$port" +ready=0 +i=0 +while [ "$i" -lt 30 ]; do + if curl -fsS -o /dev/null "$base/nginx-health" 2>/dev/null; then + ready=1 + break + fi + i=$((i + 1)) + sleep 1 +done +if [ "$ready" -ne 1 ]; then + echo 'nginx did not become ready' >&2 + docker logs "$name" >&2 || true + exit 1 +fi + +# Asset under the sub-path must serve the real file, not the SPA fallback. +asset=$(curl -fsS "$base/skillhub/assets/app.js") +if [ "$asset" != 'APP_JS_MARKER' ]; then + echo "sub-path asset must serve the real file, got: $asset" >&2 + exit 1 +fi + +# App route under the sub-path falls back to index.html (SPA). +index=$(curl -fsS "$base/skillhub/dashboard") +if [ "$index" != 'INDEX_HTML_MARKER' ]; then + echo "sub-path SPA route must serve index.html, got: $index" >&2 + exit 1 +fi + +# Bare prefix redirects to the trailing-slash form. +code=$(curl -s -o /dev/null -w '%{http_code}' "$base/skillhub") +if [ "$code" != '301' ]; then + echo "bare prefix must 301-redirect, got: $code" >&2 + exit 1 +fi + +docker rm -f "$name" >/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 +# base, not fall back to root. Reproduces the reported P1 regression. +fixed_html="$tmp/fixed-html" +mkdir -p "$fixed_html/assets" +printf '%s\n' 'INDEX_HTML_MARKER' >"$fixed_html/index.html" +printf '%s\n' 'FIXED_APP_JS_MARKER' >"$fixed_html/assets/app.js" +baked_file="$tmp/baked-base-path" +printf '%s' '/fixed/' >"$baked_file" +fixed_name="$name-fixed" +fixed_port=18081 + +docker run -d --name "$fixed_name" \ + -p "$fixed_port: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_BAKED_BASE_PATH_FILE=/etc/skillhub/baked-base-path \ + -v "$fixed_html:/usr/share/nginx/html:ro" \ + -v "$baked_file:/etc/skillhub/baked-base-path:ro" \ + -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" \ + "$NGINX_IMAGE" >/dev/null 2>&1 + +fixed_base="http://127.0.0.1:$fixed_port" +ready=0 +i=0 +while [ "$i" -lt 30 ]; do + if curl -fsS -o /dev/null "$fixed_base/nginx-health" 2>/dev/null; then + ready=1 + break + fi + i=$((i + 1)) + sleep 1 +done +if [ "$ready" -ne 1 ]; then + echo 'nginx (fixed-base) did not become ready' >&2 + docker logs "$fixed_name" >&2 || true + exit 1 +fi + +fixed_asset=$(curl -fsS "$fixed_base/fixed/assets/app.js") +if [ "$fixed_asset" != 'FIXED_APP_JS_MARKER' ]; then + echo "fixed-base asset must serve the real file, got: $fixed_asset" >&2 + exit 1 +fi + +printf '%s\n' 'web-base-path-nginx-smoke-test passed' diff --git a/scripts/tests/web-base-path-routing-test.sh b/scripts/tests/web-base-path-routing-test.sh new file mode 100755 index 00000000..97c03087 --- /dev/null +++ b/scripts/tests/web-base-path-routing-test.sh @@ -0,0 +1,135 @@ +#!/bin/sh +set -eu + +ROOT_DIR=$(CDPATH= cd -- "$(dirname -- "$0")/../.." && pwd) +ENTRYPOINT="$ROOT_DIR/web/docker-entrypoint.d/20-base-path.sh" +tmp=$(mktemp -d) +trap 'rm -rf "$tmp"' EXIT + +web_root="$tmp/html" +routing_config="$tmp/skillhub-base-path.conf" +mkdir -p "$web_root" +printf '%s\n' \ + '' \ + '/__SKILLHUB_WEB_BASE_PATH__/assets/index.js' >"$web_root/index.html" + +SKILLHUB_WEB_BASE_PATH=/skillhub/ \ +SKILLHUB_WEB_ROOT="$web_root" \ +SKILLHUB_NGINX_BASE_PATH_CONFIG="$routing_config" \ +sh "$ENTRYPOINT" + +grep -F 'location = /skillhub {' "$routing_config" >/dev/null +grep -F 'return 301 /skillhub/;' "$routing_config" >/dev/null +grep -F 'location ^~ /skillhub/ {' "$routing_config" >/dev/null +grep -F 'rewrite ^/skillhub/(.*)$ /$1 last;' "$routing_config" >/dev/null +grep -F 'set $skillhub_forwarded_prefix /skillhub;' "$routing_config" >/dev/null +grep -F '/skillhub/assets/index.js' "$web_root/index.html" >/dev/null +# favicon (and any other absolute asset ref in index.html) picks up the sub-path prefix. +grep -F '/skillhub/favicon.svg' "$web_root/index.html" >/dev/null + +# A same-origin API base that does not match the base path must be rejected at startup. +mismatch_root="$tmp/mismatch-html" +mismatch_config="$tmp/mismatch.conf" +mkdir -p "$mismatch_root" +if SKILLHUB_WEB_BASE_PATH=/skillhub/ \ + SKILLHUB_WEB_API_BASE_URL=/other \ + SKILLHUB_WEB_ROOT="$mismatch_root" \ + SKILLHUB_NGINX_BASE_PATH_CONFIG="$mismatch_config" \ + sh "$ENTRYPOINT" 2>/dev/null; then + echo 'entrypoint must reject an API base that does not match the base path' >&2 + exit 1 +fi + +if [ "$(grep -Fc 'proxy_set_header X-Forwarded-Prefix $skillhub_forwarded_prefix;' "$ROOT_DIR/web/nginx.conf.template")" -ne 4 ]; then + echo 'API, OAuth, and .well-known proxy locations must forward the configured base path' >&2 + exit 1 +fi + +root_web_root="$tmp/root-html" +root_routing_config="$tmp/root-base-path.conf" +mkdir -p "$root_web_root" +printf '%s\n' '/__SKILLHUB_WEB_BASE_PATH__/assets/index.js' >"$root_web_root/index.html" + +SKILLHUB_WEB_BASE_PATH=/ \ +SKILLHUB_WEB_ROOT="$root_web_root" \ +SKILLHUB_NGINX_BASE_PATH_CONFIG="$root_routing_config" \ +sh "$ENTRYPOINT" + +grep -F '# No sub-path routing is required for root deployment.' "$root_routing_config" >/dev/null +grep -F 'set $skillhub_forwarded_prefix "";' "$root_routing_config" >/dev/null +if grep -F 'location ' "$root_routing_config" >/dev/null; then + echo 'root deployment must not generate a duplicate Nginx location' >&2 + exit 1 +fi +grep -F '/assets/index.js' "$root_web_root/index.html" >/dev/null + +# Runtime input validation must reject '.'/'..' path segments (they normalize and +# would desync the generated location from the baked asset URLs). +for bad in '/foo/../bar/' '/foo/./bar/' '/foo//bar/' '/no-trailing' 'foo/' \ + '/api/' '/oauth2/' '/login/' '/assets/' '/registry/' '/nginx-health/' \ + '/.well-known/' '/runtime-config.js/' '/api/nested/'; do + reject_root="$tmp/reject-html" + reject_config="$tmp/reject.conf" + mkdir -p "$reject_root" + if SKILLHUB_WEB_BASE_PATH="$bad" \ + SKILLHUB_WEB_ROOT="$reject_root" \ + SKILLHUB_NGINX_BASE_PATH_CONFIG="$reject_config" \ + sh "$ENTRYPOINT" 2>/dev/null; then + echo "entrypoint must reject invalid SKILLHUB_WEB_BASE_PATH: $bad" >&2 + exit 1 + fi + rm -rf "$reject_root" "$reject_config" +done + +# A fixed-base build (no runtime SKILLHUB_WEB_BASE_PATH) must default to the baked +# base recorded at build time and generate the matching Nginx routing. +baked_root="$tmp/baked-html" +baked_config="$tmp/baked.conf" +baked_file="$tmp/baked-base-path" +mkdir -p "$baked_root" +printf '%s\n' '/skillhub/assets/index.js' >"$baked_root/index.html" +printf '%s' '/skillhub/' >"$baked_file" + +SKILLHUB_WEB_ROOT="$baked_root" \ +SKILLHUB_NGINX_BASE_PATH_CONFIG="$baked_config" \ +SKILLHUB_WEB_BAKED_BASE_PATH_FILE="$baked_file" \ +sh "$ENTRYPOINT" + +grep -F 'location ^~ /skillhub/ {' "$baked_config" >/dev/null +grep -F 'set $skillhub_forwarded_prefix /skillhub;' "$baked_config" >/dev/null + +# The bundled Compose/K8s deploy configs pass SKILLHUB_WEB_BASE_PATH as an empty +# string (unset by the operator). An empty value must be treated as "use the +# baked base", not as root — otherwise a fixed-base image serves broken assets. +empty_config="$tmp/empty.conf" +SKILLHUB_WEB_BASE_PATH= \ +SKILLHUB_WEB_ROOT="$baked_root" \ +SKILLHUB_NGINX_BASE_PATH_CONFIG="$empty_config" \ +SKILLHUB_WEB_BAKED_BASE_PATH_FILE="$baked_file" \ +sh "$ENTRYPOINT" + +grep -F 'location ^~ /skillhub/ {' "$empty_config" >/dev/null +grep -F 'set $skillhub_forwarded_prefix /skillhub;' "$empty_config" >/dev/null + +# A runtime base that conflicts with the baked base must fail loudly rather than +# silently serving mismatched routing/assets. +conflict_config="$tmp/conflict.conf" +if SKILLHUB_WEB_BASE_PATH=/other/ \ + SKILLHUB_WEB_ROOT="$baked_root" \ + SKILLHUB_NGINX_BASE_PATH_CONFIG="$conflict_config" \ + SKILLHUB_WEB_BAKED_BASE_PATH_FILE="$baked_file" \ + sh "$ENTRYPOINT" 2>/dev/null; then + echo 'entrypoint must reject a runtime base that conflicts with the baked base' >&2 + exit 1 +fi + +# An explicit runtime base matching the baked base is accepted. +match_config="$tmp/match.conf" +SKILLHUB_WEB_BASE_PATH=/skillhub/ \ +SKILLHUB_WEB_ROOT="$baked_root" \ +SKILLHUB_NGINX_BASE_PATH_CONFIG="$match_config" \ +SKILLHUB_WEB_BAKED_BASE_PATH_FILE="$baked_file" \ +sh "$ENTRYPOINT" +grep -F 'location ^~ /skillhub/ {' "$match_config" >/dev/null + +printf '%s\n' 'web-base-path-routing-test passed' diff --git a/scripts/tests/workflow-security-test.sh b/scripts/tests/workflow-security-test.sh index 62002ab5..281d4092 100755 --- a/scripts/tests/workflow-security-test.sh +++ b/scripts/tests/workflow-security-test.sh @@ -79,6 +79,10 @@ grep -Fq 'bash scripts/tests/nginx-forwarded-proto-test.sh' "$PR_SCRIPTS_WORKFLO || fail "pr-scripts must run nginx-forwarded-proto-test" grep -Fq 'bash scripts/tests/smoke-test-admin-mode-test.sh' "$PR_SCRIPTS_WORKFLOW" \ || fail "pr-scripts must run smoke-test-admin-mode-test" +grep -Fq 'bash scripts/tests/web-base-path-routing-test.sh' "$PR_SCRIPTS_WORKFLOW" \ + || fail "pr-scripts must run web-base-path-routing-test" +grep -Fq 'bash scripts/tests/web-base-path-nginx-smoke-test.sh' "$PR_SCRIPTS_WORKFLOW" \ + || fail "pr-scripts must run web-base-path-nginx-smoke-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 7549a782..eaaca6b1 100755 --- a/scripts/validate-release-config.sh +++ b/scripts/validate-release-config.sh @@ -79,6 +79,37 @@ validate_url() { http://*|https://*) ;; *) error "$var_name must start with http:// or https://" ;; esac + # A query or fragment corrupts these base URLs: they are string-concatenated + # with paths (e.g. ${SKILLHUB_PUBLIC_BASE_URL}/cli/auth), so a trailing + # ?query/#fragment would swallow the appended path. + case "$var_value" in + *'?'*|*'#'*) error "$var_name must not contain a query ('?') or fragment ('#')" ;; + esac + # Reject a scheme with no host (e.g. https://), which would concatenate into an + # invalid URL such as https:///cli/auth. + case "$var_value" in + http://*|https://*) + rest=${var_value#*://} + host=${rest%%[/?#]*} + if [ -z "$host" ]; then + error "$var_name must include a host (e.g. https://skills.example.com): $var_value" + fi + ;; + esac +} + +validate_web_api_base_url() { + var_name="$1" + eval "var_value=\${$var_name:-}" + if [ -z "$var_value" ]; then + return 0 + fi + case "$var_value" in + http://*|https://*) ;; + //*) error "$var_name must be an absolute http(s) URL or a root-relative path" ;; + /*) ;; + *) error "$var_name must be an absolute http(s) URL or a root-relative path" ;; + esac } validate_no_trailing_slash() { @@ -89,6 +120,69 @@ validate_no_trailing_slash() { esac } +validate_web_base_path_format() { + # Mirror the runtime check in web/docker-entrypoint.d/20-base-path.sh so invalid + # values are rejected here instead of only failing at container start. + value="${SKILLHUB_WEB_BASE_PATH:-}" + [ -z "$value" ] && return 0 + case "$value" in + /|/*/) ;; + *) error "SKILLHUB_WEB_BASE_PATH must be '/' or start and end with '/': $value"; return ;; + esac + case "$value" in + *//*|*[!A-Za-z0-9._~/-]*) error "SKILLHUB_WEB_BASE_PATH contains unsupported characters: $value"; return ;; + esac + case "$value" in + */./*|*/../*) error "SKILLHUB_WEB_BASE_PATH must not contain '.' or '..' path segments: $value" ;; + esac + if [ "$value" != / ]; then + first_segment=${value#/} + first_segment=${first_segment%%/*} + case "$first_segment" in + api|oauth2|login|assets|registry|nginx-health|.well-known|runtime-config.js) + error "SKILLHUB_WEB_BASE_PATH must not start with a segment reserved by the SkillHub server ($first_segment); it would shadow the server's own Nginx location: $value" + ;; + esac + fi +} + +validate_api_base_path_alignment() { + web_base_path="${SKILLHUB_WEB_BASE_PATH:-/}" + api_base="${SKILLHUB_WEB_API_BASE_URL:-}" + if [ "$web_base_path" = / ] || [ -z "$api_base" ]; then + return 0 + fi + # An absolute API URL points at a separate host and is allowed to differ. + case "$api_base" in + http://* | https://*) return 0 ;; + esac + expected="${web_base_path%/}" + if [ "$api_base" != "$expected" ]; then + error "SKILLHUB_WEB_API_BASE_URL ($api_base) must equal SKILLHUB_WEB_BASE_PATH without its trailing slash ($expected) for same-origin sub-path routing, or be an absolute URL for a separate API host" + fi +} + +validate_public_base_path_alignment() { + web_base_path="${SKILLHUB_WEB_BASE_PATH:-/}" + public_base_url="${SKILLHUB_PUBLIC_BASE_URL:-}" + if [ "$web_base_path" = / ] || [ -z "$public_base_url" ]; then + return 0 + fi + + # Compare the URL path component exactly, not just the suffix: https://host/other/skillhub + # ends with /skillhub but serves the app at /other/skillhub, which would not match. + rest="${public_base_url#*://}" + case "$rest" in + */*) public_path="/${rest#*/}" ;; + *) public_path="" ;; + esac + public_path="${public_path%/}" + required="${web_base_path%/}" + if [ "$public_path" != "$required" ]; then + error "SKILLHUB_PUBLIC_BASE_URL path ($public_path) must equal SKILLHUB_WEB_BASE_PATH without its trailing slash ($required)" + fi +} + validate_boolean() { var_name="$1" eval "var_value=\${$var_name:-}" @@ -262,10 +356,14 @@ case "$storage_provider" in esac if [ -n "${SKILLHUB_WEB_API_BASE_URL:-}" ]; then - validate_url SKILLHUB_WEB_API_BASE_URL + validate_web_api_base_url SKILLHUB_WEB_API_BASE_URL validate_no_trailing_slash SKILLHUB_WEB_API_BASE_URL fi +validate_web_base_path_format +validate_public_base_path_alignment +validate_api_base_path_alignment + if [ -n "${DEVICE_AUTH_VERIFICATION_URI:-}" ]; then validate_url DEVICE_AUTH_VERIFICATION_URI fi diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/WellKnownController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/WellKnownController.java index bc505f49..263634aa 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/WellKnownController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/WellKnownController.java @@ -1,5 +1,6 @@ package com.iflytek.skillhub.compat; +import jakarta.servlet.http.HttpServletRequest; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RestController; @@ -12,7 +13,12 @@ import java.util.Map; public class WellKnownController { @GetMapping("/.well-known/clawhub.json") - public Map clawhubConfig() { - return Map.of("apiBase", "/api/v1"); + public Map clawhubConfig(HttpServletRequest request) { + // Honor the deployment sub-path so CLI clients discover //api/v1 instead of + // the domain-root /api/v1. With forward-headers-strategy=framework, an upstream + // X-Forwarded-Prefix is reflected into the request context path. + String contextPath = request.getContextPath(); + String prefix = (contextPath == null) ? "" : contextPath; + return Map.of("apiBase", prefix + "/api/v1"); } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/WellKnownControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/WellKnownControllerTest.java index df14d907..52fb16bf 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/WellKnownControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/WellKnownControllerTest.java @@ -33,4 +33,11 @@ class WellKnownControllerTest { .andExpect(status().isOk()) .andExpect(jsonPath("$.apiBase").value("/api/v1")); } + + @Test + void clawhubConfig_includes_forwarded_prefix() throws Exception { + mockMvc.perform(get("/.well-known/clawhub.json").header("X-Forwarded-Prefix", "/skillhub")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.apiBase").value("/skillhub/api/v1")); + } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java index 8c2ff2dc..7f5154a4 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java @@ -152,7 +152,12 @@ public class SecurityConfig { ) .logout(logout -> logout .logoutUrl("/api/v1/auth/logout") - .logoutSuccessUrl("/") + // Redirect to the deployment root honoring any sub-path prefix (X-Forwarded-Prefix + // is reflected into the context path), so logout does not escape a sub-path deployment. + .logoutSuccessHandler((request, response, authentication) -> { + String contextPath = request.getContextPath(); + response.sendRedirect(((contextPath == null) ? "" : contextPath) + "/"); + }) .invalidateHttpSession(true) .deleteCookies("SESSION") ) diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginSuccessHandler.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginSuccessHandler.java index c5ba04b9..a347dcc3 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginSuccessHandler.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginSuccessHandler.java @@ -46,6 +46,10 @@ public class OAuth2LoginSuccessHandler extends SimpleUrlAuthenticationSuccessHan } String returnTo = oauthLoginFlowService.consumeReturnTo(request.getSession(false)); if (returnTo != null) { + // returnTo is a root-relative path (web client strips the base path). The redirect + // strategy (DefaultRedirectStrategy) already prepends the request context path, which + // reflects X-Forwarded-Prefix under forward-headers-strategy=framework — so the browser + // lands under the sub-path without any manual prefixing here (which would double it). getRedirectStrategy().sendRedirect(request, response, returnTo); // The default branch below clears these via super; clear here too so both paths behave consistently. clearAuthenticationAttributes(request); diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginHandlersTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginHandlersTest.java index 750d29d3..de3a8411 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginHandlersTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginHandlersTest.java @@ -72,6 +72,42 @@ class OAuth2LoginHandlersTest { assertThat(securityContext.getAuthentication().getPrincipal()).isEqualTo(principal); } + @Test + void successHandler_appliesSubPathPrefixExactlyOnce() throws Exception { + OAuthLoginFlowService oauthLoginFlowService = mock(OAuthLoginFlowService.class); + OAuth2LoginSuccessHandler handler = new OAuth2LoginSuccessHandler( + new com.iflytek.skillhub.auth.session.PlatformSessionService(), + oauthLoginFlowService + ); + MockHttpServletRequest request = new MockHttpServletRequest(); + // X-Forwarded-Prefix is reflected into the context path under forward-headers-strategy=framework. + request.setContextPath("/skillhub"); + MockHttpServletResponse response = new MockHttpServletResponse(); + HttpSession session = request.getSession(true); + session.setAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE, "/dashboard/publish"); + + var principal = new com.iflytek.skillhub.auth.rbac.PlatformPrincipal( + "user-1", "User", "user@example.com", null, "github", Set.of() + ); + Authentication authentication = new UsernamePasswordAuthenticationToken( + new DefaultOAuth2User(List.of(), Map.of("platformPrincipal", principal, "login", "user"), "login"), + null, + List.of() + ); + org.mockito.Mockito.when(oauthLoginFlowService.consumeReturnTo(org.mockito.ArgumentMatchers.any())) + .thenAnswer(invocation -> { + HttpSession currentSession = invocation.getArgument(0); + Object value = currentSession.getAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE); + currentSession.removeAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE); + return value; + }); + + handler.onAuthenticationSuccess(request, response, authentication); + + // Prefix applied exactly once by the redirect strategy — not doubled. + assertThat(response.getRedirectedUrl()).isEqualTo("/skillhub/dashboard/publish"); + } + /** * Regression test: when an unauthenticated client hits a protected API endpoint, Spring Security * caches that request. With {@code SavedRequestAwareAuthenticationSuccessHandler} the post-login diff --git a/web/Dockerfile b/web/Dockerfile index 2ed67ae0..241d6996 100644 --- a/web/Dockerfile +++ b/web/Dockerfile @@ -4,16 +4,26 @@ WORKDIR /app COPY package.json pnpm-lock.yaml ./ RUN pnpm install --frozen-lockfile COPY . . +ARG VITE_BASE_PATH=/__SKILLHUB_WEB_BASE_PATH__/ +ENV VITE_BASE_PATH=$VITE_BASE_PATH RUN pnpm build FROM nginx:alpine ENV SKILLHUB_TRUST_FORWARDED_PROTO=false +# Record a fixed build-time base so the entrypoint defaults SKILLHUB_WEB_BASE_PATH +# to it and generates matching Nginx routing without repeating the value at runtime. +# Placeholder builds (the default) intentionally write no file and default to '/'. +ARG VITE_BASE_PATH=/__SKILLHUB_WEB_BASE_PATH__/ +RUN if [ "$VITE_BASE_PATH" != "/__SKILLHUB_WEB_BASE_PATH__/" ]; then \ + mkdir -p /etc/skillhub && printf '%s' "$VITE_BASE_PATH" > /etc/skillhub/baked-base-path; \ + fi 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 COPY runtime-config.js.template /usr/share/nginx/html/runtime-config.js.template +COPY docker-entrypoint.d/20-base-path.sh /docker-entrypoint.d/20-base-path.sh COPY docker-entrypoint.d/30-runtime-config.sh /docker-entrypoint.d/30-runtime-config.sh -RUN chmod +x /docker-entrypoint.d/30-runtime-config.sh +RUN chmod +x /docker-entrypoint.d/20-base-path.sh /docker-entrypoint.d/30-runtime-config.sh EXPOSE 80 HEALTHCHECK --interval=10s --timeout=3s \ CMD wget -qO- http://127.0.0.1/nginx-health || exit 1 diff --git a/web/base-path-config.test.ts b/web/base-path-config.test.ts new file mode 100644 index 00000000..b2b2da2a --- /dev/null +++ b/web/base-path-config.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest' +import { validateBasePath } from './base-path-config' + +describe('validateBasePath', () => { + it.each(['/skillhub/', '/foo.bar/', '/a_b~c/'])('accepts %s', (value) => { + expect(validateBasePath(value)).toBe(value) + }) + + it('accepts root deployment', () => { + expect(validateBasePath('/')).toBe('/') + }) + + it.each([ + '//cdn.example/', + '/foo//bar/', + '/foo/../bar/', + '/foo/./bar/', + '/foo bar/', + 'skillhub/', + '/skillhub', + ])('rejects unsafe or malformed value %s', (value) => { + expect(() => validateBasePath(value)).toThrow(/VITE_BASE_PATH/) + }) + + it.each([ + '/api/', + '/oauth2/', + '/login/', + '/assets/', + '/registry/', + '/nginx-health/', + '/.well-known/', + '/runtime-config.js/', + '/api/nested/', + ])('rejects reserved first segment %s', (value) => { + expect(() => validateBasePath(value)).toThrow(/reserved by the SkillHub server/) + }) +}) diff --git a/web/base-path-config.ts b/web/base-path-config.ts new file mode 100644 index 00000000..96d18b0e --- /dev/null +++ b/web/base-path-config.ts @@ -0,0 +1,46 @@ +const BASE_PATH_PATTERN = /^\/[A-Za-z0-9._~/-]+\/$/ + +// First path segments reserved by the SkillHub server's own Nginx locations +// (see web/nginx.conf.template). A base path starting with any of these would +// generate a `location ^~ //` that shadows the server route and break the +// app. Kept in sync with the runtime, release-config, and Helm checks. +const RESERVED_FIRST_SEGMENTS = new Set([ + 'api', + 'oauth2', + 'login', + 'assets', + 'registry', + 'nginx-health', + '.well-known', + 'runtime-config.js', +]) + +/** + * Validates the Vite base path before it is embedded into generated asset URLs. + * Only same-origin, normalized URL paths are allowed. + */ +export function validateBasePath(value: string): string { + if (value === '/') { + return value + } + + const segments = value.split('/').filter(Boolean) + const hasDotSegment = segments.some((segment) => segment === '.' || segment === '..') + if ( + !BASE_PATH_PATTERN.test(value) + || value.includes('//') + || hasDotSegment + ) { + throw new Error( + `VITE_BASE_PATH must be '/' or a normalized root-relative path ending with '/': ${value}`, + ) + } + + if (RESERVED_FIRST_SEGMENTS.has(segments[0])) { + throw new Error( + `VITE_BASE_PATH must not start with a segment reserved by the SkillHub server (${segments[0]}); it would shadow the server's own Nginx location: ${value}`, + ) + } + + return value +} diff --git a/web/docker-entrypoint.d/20-base-path.sh b/web/docker-entrypoint.d/20-base-path.sh new file mode 100644 index 00000000..04a60ce3 --- /dev/null +++ b/web/docker-entrypoint.d/20-base-path.sh @@ -0,0 +1,117 @@ +#!/bin/sh +set -eu + +# Substitute the build-time placeholder base with SKILLHUB_WEB_BASE_PATH +# (must start and end with '/'; defaults to '/'), so one image can serve +# any sub-path via env. It also configures Nginx to strip that prefix before +# dispatching to the existing static-file and API locations. +# +# When the image was built with a fixed base (--build-arg VITE_BASE_PATH=/foo/), +# that value is recorded at build time and used as the default here, so the +# baked assets and the generated Nginx routing stay in sync without requiring +# the operator to repeat SKILLHUB_WEB_BASE_PATH at runtime. +baked_base_path_file="${SKILLHUB_WEB_BAKED_BASE_PATH_FILE:-/etc/skillhub/baked-base-path}" +baked_base_path="" +if [ -f "$baked_base_path_file" ]; then + baked_base_path=$(cat "$baked_base_path_file") +fi + +if [ -z "${SKILLHUB_WEB_BASE_PATH:-}" ]; then + # No runtime override (unset or empty): fall back to the fixed base baked at + # build time, if any. This keeps the baked assets and generated routing in sync. + SKILLHUB_WEB_BASE_PATH="$baked_base_path" +elif [ -n "$baked_base_path" ] && [ "$SKILLHUB_WEB_BASE_PATH" != "$baked_base_path" ]; then + # A fixed-base image must not be served under a different runtime prefix: the + # baked asset URLs would not match the generated Nginx routing. Fail loudly + # instead of silently serving broken static assets. + echo "SKILLHUB_WEB_BASE_PATH ($SKILLHUB_WEB_BASE_PATH) conflicts with the base path baked into this image ($baked_base_path); rebuild with a matching VITE_BASE_PATH or leave SKILLHUB_WEB_BASE_PATH unset." >&2 + exit 1 +fi +: "${SKILLHUB_WEB_BASE_PATH:=/}" + +case "$SKILLHUB_WEB_BASE_PATH" in + /|/*/) ;; + *) + echo "SKILLHUB_WEB_BASE_PATH must be '/' or start and end with '/': $SKILLHUB_WEB_BASE_PATH" >&2 + exit 1 + ;; +esac + +case "$SKILLHUB_WEB_BASE_PATH" in + *//*|*[!A-Za-z0-9._~/-]*) + echo "SKILLHUB_WEB_BASE_PATH contains unsupported characters: $SKILLHUB_WEB_BASE_PATH" >&2 + exit 1 + ;; +esac + +# Reject '.'/'..' path segments: browsers and Nginx normalize them, so the baked +# asset URLs and the generated location would diverge. Mirrors the build-time +# check in web/base-path-config.ts. +case "$SKILLHUB_WEB_BASE_PATH" in + */./*|*/../*) + echo "SKILLHUB_WEB_BASE_PATH must not contain '.' or '..' path segments: $SKILLHUB_WEB_BASE_PATH" >&2 + exit 1 + ;; +esac + +# Reject base paths whose first segment is reserved by the server's own Nginx +# locations (/api/, /oauth2/, /login/, /assets/, /registry/, /nginx-health, +# /.well-known/, /runtime-config.js). Generating `location ^~ /api/` would +# shadow the real API route and take down the whole app. Kept in sync with +# web/base-path-config.ts, validate-release-config.sh and the Helm checks. +if [ "$SKILLHUB_WEB_BASE_PATH" != / ]; then + first_segment=${SKILLHUB_WEB_BASE_PATH#/} + first_segment=${first_segment%%/*} + case "$first_segment" in + api|oauth2|login|assets|registry|nginx-health|.well-known|runtime-config.js) + echo "SKILLHUB_WEB_BASE_PATH must not start with a segment reserved by the SkillHub server ($first_segment); it would shadow the server's own Nginx location: $SKILLHUB_WEB_BASE_PATH" >&2 + exit 1 + ;; + esac +fi + +# A same-origin API base must match the base path, otherwise the front end requests +# //api/... which the sub-path routing cannot reach. Mirrors validate-release-config.sh. +# Absolute URLs (separate API host) are allowed to differ. +if [ "$SKILLHUB_WEB_BASE_PATH" != / ] && [ -n "${SKILLHUB_WEB_API_BASE_URL:-}" ]; then + case "$SKILLHUB_WEB_API_BASE_URL" in + http://* | https://*) ;; + *) + if [ "$SKILLHUB_WEB_API_BASE_URL" != "${SKILLHUB_WEB_BASE_PATH%/}" ]; then + echo "SKILLHUB_WEB_API_BASE_URL ($SKILLHUB_WEB_API_BASE_URL) must equal SKILLHUB_WEB_BASE_PATH without its trailing slash (${SKILLHUB_WEB_BASE_PATH%/}) for same-origin sub-path routing, or be an absolute URL for a separate API host" >&2 + exit 1 + fi + ;; + esac +fi + +placeholder="/__SKILLHUB_WEB_BASE_PATH__/" +root="${SKILLHUB_WEB_ROOT:-/usr/share/nginx/html}" +routing_config="${SKILLHUB_NGINX_BASE_PATH_CONFIG:-/etc/nginx/skillhub-base-path.conf}" + +if [ "$SKILLHUB_WEB_BASE_PATH" = / ]; then + printf '%s\n' \ + '# No sub-path routing is required for root deployment.' \ + 'set $skillhub_forwarded_prefix "";' \ + >"$routing_config" +else + base_path_without_trailing_slash=${SKILLHUB_WEB_BASE_PATH%/} + printf 'set $skillhub_forwarded_prefix %s;\n\nlocation = %s {\n return 301 %s/;\n}\n\nlocation ^~ %s {\n rewrite ^%s(.*)$ /$1 last;\n}\n' \ + "$base_path_without_trailing_slash" \ + "$base_path_without_trailing_slash" \ + "$base_path_without_trailing_slash" \ + "$SKILLHUB_WEB_BASE_PATH" \ + "$SKILLHUB_WEB_BASE_PATH" \ + >"$routing_config" +fi + +if ! grep -rlq "$placeholder" "$root" 2>/dev/null; then + exit 0 +fi + +escaped=$(printf '%s' "$SKILLHUB_WEB_BASE_PATH" | sed 's/[&/\]/\\&/g') + +backup_suffix='.skillhub-base-path-backup' +find "$root" -type f \( -name '*.html' -o -name '*.js' -o -name '*.css' \) \ + -exec sed -i"$backup_suffix" "s#${placeholder}#${escaped}#g" {} + +find "$root" -type f -name "*${backup_suffix}" -delete diff --git a/web/nginx.conf.template b/web/nginx.conf.template index 25db2869..b2f8cebc 100644 --- a/web/nginx.conf.template +++ b/web/nginx.conf.template @@ -21,6 +21,12 @@ server { set $proxy_x_forwarded_proto http; } + # Sub-path routing is generated by docker-entrypoint.d/20-base-path.sh. The glob + # tolerates its absence (root deployment or config-only tests); the default below + # keeps $skillhub_forwarded_prefix defined even when the file is not present. + set $skillhub_forwarded_prefix ""; + include /etc/nginx/skillhub-base-path*.conf; + location / { try_files $uri $uri/ /index.html; } @@ -31,6 +37,7 @@ server { 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; + proxy_set_header X-Forwarded-Prefix $skillhub_forwarded_prefix; } location /oauth2/ { @@ -39,6 +46,7 @@ server { 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; + proxy_set_header X-Forwarded-Prefix $skillhub_forwarded_prefix; } location /login/oauth2/ { @@ -47,6 +55,7 @@ server { 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; + proxy_set_header X-Forwarded-Prefix $skillhub_forwarded_prefix; } location /.well-known/ { @@ -55,6 +64,7 @@ server { 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; + proxy_set_header X-Forwarded-Prefix $skillhub_forwarded_prefix; } location /assets/ { diff --git a/web/src/api/client-base-path.test.ts b/web/src/api/client-base-path.test.ts new file mode 100644 index 00000000..bac1a5e5 --- /dev/null +++ b/web/src/api/client-base-path.test.ts @@ -0,0 +1,29 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('@/shared/lib/base-path', () => ({ BASE_PATH: '/skillhub/' })) +vi.mock('@/i18n/config', () => ({ default: { resolvedLanguage: 'en' } })) +vi.mock('@/shared/lib/api-error', () => ({ + ApiError: class ApiError extends Error {}, + handleApiError: vi.fn(), +})) + +import { buildApiUrl } from './client' + +describe('API base path fallback', () => { + beforeEach(() => { + Object.defineProperty(globalThis, 'window', { + configurable: true, + writable: true, + value: { __SKILLHUB_RUNTIME_CONFIG__: undefined }, + }) + }) + + it('derives the API prefix from the deployment base path when apiBaseUrl is unset', () => { + expect(buildApiUrl('/api/v1/auth/me')).toBe('/skillhub/api/v1/auth/me') + }) + + it('lets an explicit apiBaseUrl override the base path', () => { + window.__SKILLHUB_RUNTIME_CONFIG__ = { apiBaseUrl: 'https://api.example.com' } + expect(buildApiUrl('/api/v1/auth/me')).toBe('https://api.example.com/api/v1/auth/me') + }) +}) diff --git a/web/src/api/client.test.ts b/web/src/api/client.test.ts index f2d4287b..da659024 100644 --- a/web/src/api/client.test.ts +++ b/web/src/api/client.test.ts @@ -38,6 +38,7 @@ import { WEB_API_PREFIX, buildApiUrl, fetchText, + getAppBaseUrl, getDirectAuthRuntimeConfig, getSessionBootstrapRuntimeConfig, namespaceApi, @@ -108,6 +109,14 @@ describe('buildApiUrl', () => { }) }) +describe('getAppBaseUrl', () => { + it('returns the configured public application URL', () => { + window.__SKILLHUB_RUNTIME_CONFIG__ = { appBaseUrl: 'https://example.com/skillhub' } + + expect(getAppBaseUrl()).toBe('https://example.com/skillhub') + }) +}) + describe('fetchText', () => { it('applies base URL path prefixes for fetch requests', async () => { window.__SKILLHUB_RUNTIME_CONFIG__ = { apiBaseUrl: 'https://api.example.com/skill_hub' } diff --git a/web/src/api/client.ts b/web/src/api/client.ts index d701fd11..b0fe1a2e 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -1,4 +1,5 @@ import createClient from 'openapi-fetch' +import { BASE_PATH } from '@/shared/lib/base-path' import type { paths } from './generated/schema' import type { ChangePasswordRequest, @@ -82,7 +83,20 @@ function getRuntimeConfig(): RuntimeConfig { } function getApiBaseUrl(): string { - return getRuntimeConfig().apiBaseUrl ?? '' + const configured = getRuntimeConfig().apiBaseUrl + if (configured) { + return configured + } + // Default the API prefix to the deployment base path so that setting only + // SKILLHUB_WEB_BASE_PATH (e.g. /skillhub/) still routes API calls to + // /skillhub/api/... instead of /api/... behind a sub-path-only reverse proxy. + // BASE_PATH is import.meta.env.BASE_URL (substituted at container start) and + // always ends with '/'; drop it so requests are /skillhub/api, not /skillhub//api. + return BASE_PATH === '/' ? '' : BASE_PATH.replace(/\/+$/, '') +} + +export function getAppBaseUrl(): string { + return getRuntimeConfig().appBaseUrl ?? '' } function parseBooleanFlag(value: string | undefined): boolean { @@ -381,7 +395,7 @@ export const authApi = { }, async logout(): Promise { - const response = await fetch('/api/v1/auth/logout', { + const response = await fetch(buildApiUrl('/api/v1/auth/logout'), { method: 'POST', headers: withCsrf(), }) diff --git a/web/src/app/router.tsx b/web/src/app/router.tsx index 4280e4ca..440fa56a 100644 --- a/web/src/app/router.tsx +++ b/web/src/app/router.tsx @@ -485,6 +485,7 @@ const routeTree = rootRoute.addChildren([ export const router = createRouter({ routeTree, + basepath: import.meta.env.BASE_URL, defaultNotFoundComponent: DefaultNotFound, defaultErrorComponent: RouteError, }) diff --git a/web/src/features/auth/login-button.tsx b/web/src/features/auth/login-button.tsx index cde4453f..a452a3a5 100644 --- a/web/src/features/auth/login-button.tsx +++ b/web/src/features/auth/login-button.tsx @@ -1,5 +1,6 @@ import { useTranslation } from 'react-i18next' import { Button } from '@/shared/ui/button' +import { withBasePath } from '@/shared/lib/base-path' import { useAuthMethods } from './use-auth-methods' interface LoginButtonProps { @@ -13,7 +14,7 @@ function OAuthIcon({ provider }: { provider: string }) { const normalizedProvider = provider.toLowerCase() return ( {provider} @@ -48,7 +49,7 @@ export function LoginButton({ returnTo }: LoginButtonProps) { className="w-full h-12 text-base" variant="outline" onClick={() => { - window.location.href = provider.actionUrl + window.location.href = withBasePath(provider.actionUrl) }} > diff --git a/web/src/features/notification/use-notification-sse.ts b/web/src/features/notification/use-notification-sse.ts index 6a6b17ce..5325651c 100644 --- a/web/src/features/notification/use-notification-sse.ts +++ b/web/src/features/notification/use-notification-sse.ts @@ -1,11 +1,11 @@ import { useEffect, useRef } from 'react' import type { QueryClient } from '@tanstack/react-query' import { useQueryClient } from '@tanstack/react-query' -import { WEB_API_PREFIX } from '@/api/client' +import { buildApiUrl, WEB_API_PREFIX } from '@/api/client' import { incrementUnreadCount } from './notification-unread-cache' import { createNotificationSseConnection } from './notification-sse-coordinator' -const SSE_URL = `${WEB_API_PREFIX}/notifications/sse` +const SSE_URL = buildApiUrl(`${WEB_API_PREFIX}/notifications/sse`) type NotificationSseConnectionLike = ReturnType diff --git a/web/src/features/skill/install-command.tsx b/web/src/features/skill/install-command.tsx index 3f409b9d..2886e217 100644 --- a/web/src/features/skill/install-command.tsx +++ b/web/src/features/skill/install-command.tsx @@ -4,6 +4,7 @@ import { Check, Copy } from 'lucide-react' import { Button } from '@/shared/ui/button' import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/shared/ui/tabs' import { useCopyToClipboard } from '@/shared/lib/clipboard' +import { resolvePublicRegistryUrl } from '@/shared/lib/registry-url' interface InstallCommandProps { namespace: string @@ -20,13 +21,10 @@ export function getBaseUrl(): string { return '' } const runtimeConfig = window.__SKILLHUB_RUNTIME_CONFIG__ - const configuredUrl = runtimeConfig?.appBaseUrl - // Use configured URL only if it's set and not localhost - if (configuredUrl && !configuredUrl.includes('localhost')) { - return configuredUrl - } - // Fallback to current page origin - return `${window.location.protocol}//${window.location.host}` + return resolvePublicRegistryUrl( + runtimeConfig?.appBaseUrl, + `${window.location.protocol}//${window.location.host}`, + ) } export function buildInstallCommand(namespace: string, slug: string, baseUrl: string): string { diff --git a/web/src/pages/cli-auth.test.ts b/web/src/pages/cli-auth.test.ts index 0f6cdf24..a1611850 100644 --- a/web/src/pages/cli-auth.test.ts +++ b/web/src/pages/cli-auth.test.ts @@ -27,6 +27,7 @@ vi.mock('@/shared/ui/button', () => ({ })) vi.mock('@/api/client', () => ({ + getAppBaseUrl: vi.fn().mockReturnValue(''), getCurrentUser: vi.fn().mockResolvedValue(null), tokenApi: { createToken: vi.fn() }, })) @@ -35,7 +36,19 @@ vi.mock('@/app/router', () => ({ ORIGINAL_URL_SEARCH: '', })) -import { CliAuthPage } from './cli-auth' +import { CliAuthPage, resolveCliRegistryUrl } from './cli-auth' + +describe('resolveCliRegistryUrl', () => { + it('uses the configured public base URL for the CLI registry', () => { + expect(resolveCliRegistryUrl('https://example.com/skillhub', 'https://example.com', '/skillhub/')) + .toBe('https://example.com/skillhub') + }) + + it('falls back to the browser origin plus the Vite base path', () => { + expect(resolveCliRegistryUrl('', 'https://example.com', '/skillhub/')) + .toBe('https://example.com/skillhub') + }) +}) describe('CliAuthPage', () => { it('exports a named component function', () => { diff --git a/web/src/pages/cli-auth.tsx b/web/src/pages/cli-auth.tsx index 73679948..a6be44a4 100644 --- a/web/src/pages/cli-auth.tsx +++ b/web/src/pages/cli-auth.tsx @@ -3,9 +3,11 @@ import { useNavigate } from '@tanstack/react-router' import { useTranslation } from 'react-i18next' import { Card } from '@/shared/ui/card' import { Button } from '@/shared/ui/button' -import { getCurrentUser, tokenApi } from '@/api/client' +import { getAppBaseUrl, getCurrentUser, tokenApi } from '@/api/client' import type { User } from '@/api/types' import { ORIGINAL_URL_SEARCH } from '@/app/router' +import { BASE_PATH } from '@/shared/lib/base-path' +import { resolvePublicRegistryUrl } from '@/shared/lib/registry-url' // Parse the original URL params captured before TanStack Router rewrites const ORIGINAL_PARAMS = new URLSearchParams(ORIGINAL_URL_SEARCH) @@ -34,6 +36,10 @@ function decodeLabel(labelB64?: string, labelPlain?: string): string { return labelPlain || 'CLI token' } +export function resolveCliRegistryUrl(appBaseUrl: string | undefined, origin: string, basePath = BASE_PATH): string { + return resolvePublicRegistryUrl(appBaseUrl, origin, basePath) +} + export function CliAuthPage() { const { t } = useTranslation() const navigate = useNavigate() @@ -113,7 +119,7 @@ export function CliAuthPage() { setStatus('redirecting') // Construct redirect URL with token in hash fragment - const registryUrl = window.location.origin + const registryUrl = resolveCliRegistryUrl(getAppBaseUrl(), window.location.origin) const hashParams = new URLSearchParams() hashParams.set('token', response.token) hashParams.set('registry', registryUrl) diff --git a/web/src/pages/search.tsx b/web/src/pages/search.tsx index fded8a67..a9426b55 100644 --- a/web/src/pages/search.tsx +++ b/web/src/pages/search.tsx @@ -12,6 +12,7 @@ import { Pagination } from '@/shared/components/pagination' import { useSearchSkills } from '@/shared/hooks/use-skill-queries' import { useVisibleLabels } from '@/shared/hooks/use-label-queries' import { useMyStars } from '@/shared/hooks/use-user-queries' +import { toRouterPath } from '@/shared/lib/base-path' import { formatNamespaceSearchInput, normalizeSearchQuery, parseNamespaceSearchInput } from '@/shared/lib/search-query' import { Button } from '@/shared/ui/button' import { APP_SHELL_PAGE_CLASS_NAME } from '@/app/page-shell-style' @@ -187,7 +188,7 @@ export function SearchPage() { navigate({ to: '/login', search: { - returnTo: `${window.location.pathname}${window.location.search}${window.location.hash}`, + returnTo: toRouterPath(window.location.pathname, window.location.search, window.location.hash), }, }) return @@ -197,7 +198,10 @@ export function SearchPage() { } const handleSkillClick = (namespace: string, slug: string) => { - navigate({ to: `/space/${namespace}/${encodeURIComponent(slug)}`, search: { returnTo: `${window.location.pathname}${window.location.search}` } }) + navigate({ + to: `/space/${namespace}/${encodeURIComponent(slug)}`, + search: { returnTo: toRouterPath(window.location.pathname, window.location.search) }, + }) } const filteredStarredSkills = starredOnly diff --git a/web/src/shared/components/landing-quick-start.tsx b/web/src/shared/components/landing-quick-start.tsx index d290dbb0..77a57959 100644 --- a/web/src/shared/components/landing-quick-start.tsx +++ b/web/src/shared/components/landing-quick-start.tsx @@ -3,6 +3,7 @@ import { useTranslation } from 'react-i18next' import { Bot, Check, Copy, Terminal, UserRound } from 'lucide-react' import type { LucideIcon } from 'lucide-react' import { useCopyToClipboard } from '@/shared/lib/clipboard' +import { resolvePublicRegistryUrl } from '@/shared/lib/registry-url' type LandingQuickStartTabId = 'agent' | 'human' | 'cli' @@ -29,13 +30,10 @@ function getAppBaseUrl(): string { return '' } const runtimeConfig = window.__SKILLHUB_RUNTIME_CONFIG__ - const configuredUrl = runtimeConfig?.appBaseUrl - // Use configured URL only if it's set and not localhost - if (configuredUrl && !configuredUrl.includes('localhost')) { - return configuredUrl - } - // Fallback to current page origin - return `${window.location.protocol}//${window.location.host}` + return resolvePublicRegistryUrl( + runtimeConfig?.appBaseUrl, + `${window.location.protocol}//${window.location.host}`, + ) } function CompactCopyButton({ text }: { text: string }) { diff --git a/web/src/shared/components/quick-start.tsx b/web/src/shared/components/quick-start.tsx index daedcb05..dcebfde7 100644 --- a/web/src/shared/components/quick-start.tsx +++ b/web/src/shared/components/quick-start.tsx @@ -2,19 +2,17 @@ import { useTranslation } from 'react-i18next' import { Check, Copy, Settings, Download, Upload } from 'lucide-react' import { useMemo } from 'react' import { useCopyToClipboard } from '@/shared/lib/clipboard' +import { resolvePublicRegistryUrl } from '@/shared/lib/registry-url' function getAppBaseUrl(): string { if (typeof window === 'undefined') { return 'https://skill.xfyun.cn' } const runtimeConfig = (window as unknown as Record).__SKILLHUB_RUNTIME_CONFIG__ as { appBaseUrl?: string } | undefined - const configuredUrl = runtimeConfig?.appBaseUrl - // Use configured URL only if it's set and not localhost - if (configuredUrl && !configuredUrl.includes('localhost')) { - return configuredUrl - } - // Fallback to current page origin - return `${window.location.protocol}//${window.location.host}` + return resolvePublicRegistryUrl( + runtimeConfig?.appBaseUrl, + `${window.location.protocol}//${window.location.host}`, + ) } function CopyButton({ text }: { text: string }) { diff --git a/web/src/shared/components/user-menu.tsx b/web/src/shared/components/user-menu.tsx index d3eb3d39..774f1bab 100644 --- a/web/src/shared/components/user-menu.tsx +++ b/web/src/shared/components/user-menu.tsx @@ -7,6 +7,7 @@ import { useMyNamespaces } from '@/shared/hooks/use-namespace-queries' import { buildGlobalReviewsPath, canAccessReviewCenter } from '@/features/review/review-paths' import { clearSessionScopedQueries } from '@/features/notification/notification-session' import { canViewGovernanceCenter } from '@/shared/lib/governance-access' +import { withBasePath } from '@/shared/lib/base-path' import { cn } from '@/shared/lib/utils' interface User { @@ -81,7 +82,7 @@ export function UserMenu({ user, triggerClassName }: UserMenuProps) { // Always clear cache and redirect, even if API call fails clearSessionScopedQueries(queryClient) queryClient.setQueryData(['auth', 'me'], null) - window.location.href = '/' + window.location.href = withBasePath('/') } } diff --git a/web/src/shared/lib/api-error.ts b/web/src/shared/lib/api-error.ts index 03efebca..218f38b1 100644 --- a/web/src/shared/lib/api-error.ts +++ b/web/src/shared/lib/api-error.ts @@ -1,5 +1,6 @@ import i18n from '@/i18n/config' import { toast } from './toast' +import { withBasePath } from './base-path' const ACCOUNT_DISABLED_REASON = 'accountDisabled' @@ -59,11 +60,11 @@ export function handleApiError(error: unknown): void { if (status === 401) { if (isAccountDisabledError(error)) { - window.location.href = `/login?reason=${ACCOUNT_DISABLED_REASON}` + window.location.href = withBasePath(`/login?reason=${ACCOUNT_DISABLED_REASON}`) return } toast.error(i18n.t('apiError.unauthorized')) - window.location.href = '/login' + window.location.href = withBasePath('/login') return } diff --git a/web/src/shared/lib/base-path.test.ts b/web/src/shared/lib/base-path.test.ts new file mode 100644 index 00000000..32280244 --- /dev/null +++ b/web/src/shared/lib/base-path.test.ts @@ -0,0 +1,13 @@ +import { describe, expect, it } from 'vitest' +import { toRouterPath } from './base-path' + +describe('toRouterPath', () => { + it('removes the deployment base path while preserving search and hash', () => { + expect(toRouterPath('/skillhub/search', '?q=java', '#results', '/skillhub/')).toBe('/search?q=java#results') + }) + + it('keeps root deployments and unrelated paths unchanged', () => { + expect(toRouterPath('/search', '?q=java', '#results', '/')).toBe('/search?q=java#results') + expect(toRouterPath('/skillhub-admin/search', '', '', '/skillhub/')).toBe('/skillhub-admin/search') + }) +}) diff --git a/web/src/shared/lib/base-path.ts b/web/src/shared/lib/base-path.ts new file mode 100644 index 00000000..d5e75a49 --- /dev/null +++ b/web/src/shared/lib/base-path.ts @@ -0,0 +1,36 @@ +/** + * Deployment base path (Vite `base`, always ends with '/'). Router and asset + * URLs honor it automatically; use `withBasePath` for the few full-page + * navigations (`window.location.href`) that bypass the router. + */ +export const BASE_PATH = import.meta.env.BASE_URL + +/** + * Converts a browser-visible location to the internal path expected by + * TanStack Router. Browser locations include the deployment base path, while + * Router targets must not. + */ +export function toRouterPath(pathname: string, search = '', hash = '', basePath = BASE_PATH): string { + const normalizedBasePath = basePath === '/' + ? '' + : basePath.endsWith('/') + ? basePath.slice(0, -1) + : basePath + const routerPathname = normalizedBasePath + && (pathname === normalizedBasePath || pathname.startsWith(`${normalizedBasePath}/`)) + ? pathname.slice(normalizedBasePath.length) || '/' + : pathname + + return `${routerPathname}${search}${hash}` +} + +/** + * Prefixes a root-relative path with the base path. Absolute and + * protocol-relative URLs are returned unchanged. + */ +export function withBasePath(path: string): string { + if (!path.startsWith('/') || path.startsWith('//')) { + return path + } + return `${BASE_PATH}${path.slice(1)}` +} diff --git a/web/src/shared/lib/registry-url.test.ts b/web/src/shared/lib/registry-url.test.ts new file mode 100644 index 00000000..b031bf31 --- /dev/null +++ b/web/src/shared/lib/registry-url.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from 'vitest' +import { resolvePublicRegistryUrl } from './registry-url' + +describe('resolvePublicRegistryUrl', () => { + it('keeps the deployment base path when the runtime public URL is unavailable', () => { + expect(resolvePublicRegistryUrl('', 'https://registry.example.com', '/skillhub/')) + .toBe('https://registry.example.com/skillhub') + }) + + it('uses the browser origin plus base path when the configured URL is localhost', () => { + expect(resolvePublicRegistryUrl('http://localhost:3000', 'https://registry.example.com', '/skillhub/')) + .toBe('https://registry.example.com/skillhub') + }) + + it('uses a configured non-localhost public URL without a trailing slash', () => { + expect(resolvePublicRegistryUrl('https://registry.example.com/skillhub/', 'https://ignored.example.com', '/skillhub/')) + .toBe('https://registry.example.com/skillhub') + }) +}) diff --git a/web/src/shared/lib/registry-url.ts b/web/src/shared/lib/registry-url.ts new file mode 100644 index 00000000..67bc8f1a --- /dev/null +++ b/web/src/shared/lib/registry-url.ts @@ -0,0 +1,23 @@ +import { BASE_PATH } from './base-path' + +function withoutTrailingSlash(value: string): string { + return value === '/' ? '' : value.replace(/\/+$/, '') +} + +/** + * Resolves the public registry URL used in copied CLI and agent commands. + * Runtime configuration wins outside local development; otherwise the current + * browser origin must retain Vite's deployment base path. + */ +export function resolvePublicRegistryUrl( + appBaseUrl: string | undefined, + origin: string, + basePath = BASE_PATH, +): string { + const configuredUrl = appBaseUrl?.trim() + if (configuredUrl && !configuredUrl.includes('localhost')) { + return withoutTrailingSlash(configuredUrl) + } + + return `${withoutTrailingSlash(origin)}${withoutTrailingSlash(basePath)}` +} diff --git a/web/src/shared/lib/role-guard.test.ts b/web/src/shared/lib/role-guard.test.ts index 62ca5129..68b1e61b 100644 --- a/web/src/shared/lib/role-guard.test.ts +++ b/web/src/shared/lib/role-guard.test.ts @@ -49,4 +49,13 @@ describe('buildLoginRedirect', () => { }, }) }) + + it('converts a sub-path browser location to a Router path', () => { + expect(buildLoginRedirect('/skillhub/dashboard/reviews/13', '?tab=pending', '#panel', '/skillhub/')).toEqual({ + to: '/login', + search: { + returnTo: '/dashboard/reviews/13?tab=pending#panel', + }, + }) + }) }) diff --git a/web/src/shared/lib/role-guard.ts b/web/src/shared/lib/role-guard.ts index d265d20f..7dded77c 100644 --- a/web/src/shared/lib/role-guard.ts +++ b/web/src/shared/lib/role-guard.ts @@ -1,3 +1,5 @@ +import { toRouterPath } from './base-path' + export function canAccessRoute(userRoles: readonly string[] | undefined, requiredRoles: readonly string[]) { if (!userRoles || userRoles.length === 0) { return false @@ -14,11 +16,11 @@ export function shouldRedirectToLogin(isLoading: boolean, user: object | null | return !isLoading && !user } -export function buildLoginRedirect(pathname: string, search = '', hash = '') { +export function buildLoginRedirect(pathname: string, search = '', hash = '', basePath?: string) { return { to: '/login' as const, search: { - returnTo: `${pathname}${search}${hash}`, + returnTo: toRouterPath(pathname, search, hash, basePath), }, } } diff --git a/web/src/vite-env.d.ts b/web/src/vite-env.d.ts new file mode 100644 index 00000000..11f02fe2 --- /dev/null +++ b/web/src/vite-env.d.ts @@ -0,0 +1 @@ +/// diff --git a/web/vite.config.ts b/web/vite.config.ts index c5abfe3b..32566d1d 100644 --- a/web/vite.config.ts +++ b/web/vite.config.ts @@ -1,11 +1,13 @@ import { defineConfig } from 'vite' import react from '@vitejs/plugin-react' import path from 'path' +import { validateBasePath } from './base-path-config' const JS_BUILD_TARGET = 'es2020' const LEGACY_BROWSER_TARGETS = ['chrome83', 'edge83', 'firefox78', 'safari14'] export default defineConfig({ + base: validateBasePath(process.env.VITE_BASE_PATH ?? '/'), plugins: [react()], resolve: { alias: {