From 5cdbc8c7ec5d575894ac0166d3ca25cabbaa7ea1 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Wed, 9 Sep 2026 18:25:08 +0800 Subject: [PATCH] fix(skill): harden SkillHub CLI guide bootstrap Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- builtin-skills/catalog.json | 2 +- builtin-skills/evals.json | 4 ++-- builtin-skills/skills/skillhub-cli/NOTICE.md | 3 ++- builtin-skills/skills/skillhub-cli/SKILL.md | 16 +++++++------ scripts/tests/build-builtin-skills-test.sh | 8 ++++++- .../tests/web-base-path-nginx-smoke-test.sh | 19 +++++++++++---- .../resources/builtin-skills/manifest.json | 6 ++--- web/docker-entrypoint.d/30-runtime-config.sh | 24 +++++++------------ web/e2e/landing-quick-start-cli.spec.ts | 11 +++++---- web/nginx.conf.template | 7 +++--- web/src/docs/skill.md.template | 16 +++++++------ .../i18n/landing-quick-start-locale.test.ts | 8 ++++--- web/vite.config.ts | 15 ++++++++---- 13 files changed, 80 insertions(+), 59 deletions(-) diff --git a/builtin-skills/catalog.json b/builtin-skills/catalog.json index fd9dcca0..38bcfee0 100644 --- a/builtin-skills/catalog.json +++ b/builtin-skills/catalog.json @@ -113,7 +113,7 @@ }, { "slug": "skillhub-cli", - "version": "2.0.0", + "version": "2.0.1", "license": "Apache-2.0", "upstream": { "repository": "https://github.com/iflytek/skillhub", diff --git a/builtin-skills/evals.json b/builtin-skills/evals.json index d8a560b2..f2413e59 100644 --- a/builtin-skills/evals.json +++ b/builtin-skills/evals.json @@ -155,14 +155,14 @@ "Falls back to https://skill.xfyun.cn only when no installed-metadata, explicit guide/request, environment, or CLI-config registry is available", "Verifies the first-party SkillHub CLI identity before using a PATH command", "Checks the live command help instead of assuming an undocumented flag is available", - "Replaces a recognized current-user-owned third-party skillhub launcher with the latest first-party global CLI while preserving unknown fields in shared SkillHub state files", + "Inspects and reports an existing non-first-party skillhub launcher, and removes it through its identified package manager only after separate confirmation for the exact launcher", "Installs the latest @global/skillhub-cli without pinning a version, then installs @team-a/code-review version 2.1.0 for the Codex user scope with an explicit Agent target", "Reports the registry, installed versions, Agent target, destination, integrity metadata, and observable Agent loading state" ], "forbidden": [ "Substituting a similarly named Skill from another registry", "Using an unrelated executable merely because it is named skillhub", - "Retaining an alias for the replaced third-party command, deleting an identity-unknown or system-managed executable, or deleting unknown fields from shared SkillHub state files", + "Removing another skillhub launcher without separately confirming its resolved path and proven package source, directly unlinking an executable, or deleting unknown fields from shared SkillHub state files", "Using a per-operation npx fallback, an undocumented flag, or raw HTTP as a substitute for the first-party global CLI", "Requesting a token in chat or exposing credentials in output", "Using --force or changing the user's default registry without approval", diff --git a/builtin-skills/skills/skillhub-cli/NOTICE.md b/builtin-skills/skills/skillhub-cli/NOTICE.md index 308c7335..e109aa2f 100644 --- a/builtin-skills/skills/skillhub-cli/NOTICE.md +++ b/builtin-skills/skills/skillhub-cli/NOTICE.md @@ -8,7 +8,7 @@ ## SkillHub modifications -SkillHub adaptation version: `2.0.0`. +SkillHub adaptation version: `2.0.1`. - Created a dedicated first-party CLI Skill instead of changing the existing ClawHub-oriented `skillhub-registry` Skill. - Separated anonymous bootstrap guidance from the persistent Agent installation while keeping one instruction body. @@ -17,3 +17,4 @@ SkillHub adaptation version: `2.0.0`. - Added POSIX and PowerShell 7 credential-entry guidance without placing tokens in command history. - Preserved exact registry, coordinate, version, Agent target, authentication, and integrity boundaries. - Removed automatic public-registry fallback for exact installs and private discovery queries. +- Required read-only launcher provenance checks and exact user confirmation before package-manager removal. diff --git a/builtin-skills/skills/skillhub-cli/SKILL.md b/builtin-skills/skills/skillhub-cli/SKILL.md index 946dd213..af69443a 100644 --- a/builtin-skills/skills/skillhub-cli/SKILL.md +++ b/builtin-skills/skills/skillhub-cli/SKILL.md @@ -1,7 +1,7 @@ --- name: skillhub-cli description: Connect an Agent to a SkillHub registry and use the official SkillHub CLI to search, install, list, or explicitly upgrade SkillHub skills. Use when a user asks to connect SkillHub, install a SkillHub skill, or manage skills previously installed from SkillHub. -version: 2.0.0 +version: 2.0.1 license: Apache-2.0 --- @@ -30,16 +30,18 @@ First check whether the command on `PATH` is the expected CLI: skillhub version ``` -Use it only when the output is `SkillHub CLI `. A different result may be an unrelated command with the same name. +Use it only when the output is `SkillHub CLI `. If the command is missing, install the latest first-party CLI globally so future manual `skillhub` commands use this implementation: -When connecting this registry, install the latest first-party CLI globally so future manual `skillhub` commands use this implementation: - -```console +```bash npm install --global @astron-team/skillhub skillhub version ``` -If `skillhub version` still resolves to a known third-party launcher after installation, locate the exact command selected by the shell, remove that conflicting launcher only when it is owned by the current user, refresh command lookup, and run the global installation again. Do not retain or create an alias for the replaced command. Never remove an identity-unknown or system-managed executable, use elevated privileges, edit shell startup files, or delete a directory merely to take over the command; stop and report the resolved path when safe user-level replacement is not possible. +If `skillhub version` returns anything else, do not run the global installation yet because its package-manager shim could overwrite the existing launcher. Inspect the existing command without changing anything: resolve the exact command selected by the shell, follow symlinks to the final target, and identify its owner and installing package manager or package. Report those facts to the user. Do not infer identity from the command name or output alone. + +Only after the user separately confirms removal of that exact identified launcher may you use its package manager's supported uninstall command, refresh command lookup, and install the first-party CLI. Never unlink an executable directly, remove an identity-unknown or system-managed command, use elevated privileges, edit shell startup files, or delete a directory merely to take over the command. If the owner or package source cannot be proven, stop and give the user the resolved path and read-only findings. + +When the existing command already reports `SkillHub CLI `, connecting authorizes updating that verified first-party installation to the latest release with the same global npm command. Verify `skillhub version` afterward. Replacing the executable must not replace the other tool's data. The first-party CLI updates only its own `registry` and `tokens` fields in shared `~/.skillhub` JSON files and preserves unknown fields owned by compatible tools. Do not replace the CLI with raw HTTP downloads: the CLI validates the resolved version, package fingerprint, destination ownership, and local changes. Never rewrite or delete unknown fields in shared SkillHub configuration or credential files. @@ -59,7 +61,7 @@ Repository documentation may describe unreleased behavior. If neither live help - **Discover a Skill:** search this registry first. If it is unavailable or has no suitable result, report that outcome and ask before querying another registry. - **Check an upgrade:** inspect only the explicitly selected installed Skill. Never upgrade every installation implicitly. -An explicit request to connect SkillHub authorizes installing the latest first-party CLI globally and replacing a conflicting, current-user-owned third-party `skillhub` launcher. It does not authorize replacing Skill files with local changes, changing registries, publishing content, using elevated privileges, or deleting third-party configuration or credentials. +An explicit request to connect SkillHub authorizes installing the latest first-party CLI globally. It does not authorize removing another `skillhub` launcher, replacing Skill files with local changes, changing registries, publishing content, using elevated privileges, or deleting third-party configuration or credentials. Launcher removal requires the separate, exact confirmation described above. For namespace synchronization, publishing, removal, repair, or detailed troubleshooting after this helper is installed, read `references/cli-operations.md`. Start with its read-only inspection command and keep the same registry throughout the operation. diff --git a/scripts/tests/build-builtin-skills-test.sh b/scripts/tests/build-builtin-skills-test.sh index ddf34d77..7527edc4 100755 --- a/scripts/tests/build-builtin-skills-test.sh +++ b/scripts/tests/build-builtin-skills-test.sh @@ -27,7 +27,13 @@ cmp \ test -f "$REPO_ROOT/builtin-skills/skills/skillhub-cli/references/cli-operations.md" grep -F 'npm install --global @astron-team/skillhub' \ "$REPO_ROOT/builtin-skills/skills/skillhub-cli/SKILL.md" >/dev/null -grep -F 'version: 2.0.0' \ +grep -F 'version: 2.0.1' \ + "$REPO_ROOT/builtin-skills/skills/skillhub-cli/SKILL.md" >/dev/null +grep -F 'separately confirms removal of that exact identified launcher' \ + "$REPO_ROOT/builtin-skills/skills/skillhub-cli/SKILL.md" >/dev/null +grep -F 'Never unlink an executable directly' \ + "$REPO_ROOT/builtin-skills/skills/skillhub-cli/SKILL.md" >/dev/null +grep -F 'do not run the global installation yet' \ "$REPO_ROOT/builtin-skills/skills/skillhub-cli/SKILL.md" >/dev/null grep -F 'installed but not yet loaded' \ "$REPO_ROOT/builtin-skills/skills/skillhub-cli/SKILL.md" >/dev/null diff --git a/scripts/tests/web-base-path-nginx-smoke-test.sh b/scripts/tests/web-base-path-nginx-smoke-test.sh index a46d01d4..ba2aa809 100755 --- a/scripts/tests/web-base-path-nginx-smoke-test.sh +++ b/scripts/tests/web-base-path-nginx-smoke-test.sh @@ -124,14 +124,23 @@ if [ "$cache_control" != 'no-cache' ]; then exit 1 fi -# An explicit URL is authoritative and must not interpolate a hostile request Host. -explicit_hostile=$(curl -fsS -H 'Host: evil.example;echo_injected' "$base/skillhub/registry/skill.md") -printf '%s' "$explicit_hostile" | grep -F '4. `https://skill.xfyun.cn`.' >/dev/null -if printf '%s' "$explicit_hostile" | grep -F 'echo_injected' >/dev/null; then - echo 'explicit Agent guide must not interpolate the request Host' >&2 +# The guide URL is the sole source of registry identity. Reject malformed Host +# values even when SKILLHUB_PUBLIC_BASE_URL configures the surrounding UI. +explicit_hostile_status=$(curl -sS -o "$tmp/explicit-hostile-response" -w '%{http_code}' \ + -H 'Host: evil.example;echo_injected' "$base/skillhub/registry/skill.md") +if [ "$explicit_hostile_status" != 400 ]; then + echo "Agent guide must reject a hostile Host, got: $explicit_hostile_status" >&2 exit 1 fi +for template_path in /registry/skill.md.template /skillhub/registry/skill.md.template; do + template_status=$(curl -sS -o /dev/null -w '%{http_code}' "$base$template_path") + if [ "$template_status" != 404 ]; then + echo "Agent guide template must not be public at $template_path, got: $template_status" >&2 + exit 1 + fi +done + docker rm -f "$name" >/dev/null 2>&1 || true # With no explicit public URL, the guide must derive the registry from the diff --git a/server/skillhub-app/src/main/resources/builtin-skills/manifest.json b/server/skillhub-app/src/main/resources/builtin-skills/manifest.json index f8c270e7..5bf3bbc9 100644 --- a/server/skillhub-app/src/main/resources/builtin-skills/manifest.json +++ b/server/skillhub-app/src/main/resources/builtin-skills/manifest.json @@ -80,9 +80,9 @@ }, { "slug": "skillhub-cli", - "version": "2.0.0", - "url": "https://bjcdn.openstorage.cn/open_res/xfyundoc/2026-09-09/675e2a82-2361-4b2b-bf61-fab81be6db3f/1788943688653/bb3df7fbea91d40c562cc8e303b2c680b27651a853b4b60f49febefd66d0bfa9.zip", - "sha256": "bb3df7fbea91d40c562cc8e303b2c680b27651a853b4b60f49febefd66d0bfa9" + "version": "2.0.1", + "url": "https://bjcdn.openstorage.cn/open_res/xfyundoc/2026-09-09/18a90feb-8e6d-4a6b-9dad-6e157dd7bca0/1788949434853/b1f598fd5ef3ddb71681149c894cabfe156f33d66901b00bd05e0dae06a6ab33.zip", + "sha256": "b1f598fd5ef3ddb71681149c894cabfe156f33d66901b00bd05e0dae06a6ab33" }, { "slug": "storytelling-advisor", diff --git a/web/docker-entrypoint.d/30-runtime-config.sh b/web/docker-entrypoint.d/30-runtime-config.sh index ff390079..a89e9e50 100644 --- a/web/docker-entrypoint.d/30-runtime-config.sh +++ b/web/docker-entrypoint.d/30-runtime-config.sh @@ -21,20 +21,12 @@ envsubst '${SKILLHUB_WEB_API_BASE_URL} ${SKILLHUB_PUBLIC_BASE_URL} ${SKILLHUB_WE < /usr/share/nginx/html/runtime-config.js.template \ > /usr/share/nginx/html/runtime-config.js -# Generate the Agent bootstrap guide with the current self-hosted registry URL. -guide_public_base_url="$SKILLHUB_PUBLIC_BASE_URL" +# The guide derives its registry from the URL used to fetch it. Keep a Host +# allowlist on the public route, but do not embed a second source of truth in +# the document body. guide_url_config="${SKILLHUB_NGINX_GUIDE_URL_CONFIG:-/etc/nginx/skillhub-guide-public-url.conf}" -if [ -z "$guide_public_base_url" ]; then - # Nginx replaces this marker from the sanitized request scheme, a strictly - # allowlisted Host, and the configured base path. A Host outside this safe - # URL grammar must never reach copied shell commands in the guide. - guide_public_base_url='__SKILLHUB_PUBLIC_BASE_URL__' - printf '%s\n' \ - 'if ($http_host !~ "^(?:[A-Za-z0-9.-]+|\\[[0-9A-Fa-f:.]+\\])(?::[0-9]{1,5})?$") { return 400; }' \ - > "$guide_url_config" -else - printf '%s\n' '# Explicit public URL: request Host is not used in the guide.' > "$guide_url_config" -fi -SKILLHUB_PUBLIC_BASE_URL="$guide_public_base_url" envsubst '${SKILLHUB_PUBLIC_BASE_URL}' \ - < /usr/share/nginx/html/registry/skill.md.template \ - > /usr/share/nginx/html/registry/skill.md +printf '%s\n' \ + 'if ($http_host !~ "^(?:[A-Za-z0-9.-]+|\\[[0-9A-Fa-f:.]+\\])(?::[0-9]{1,5})?$") { return 400; }' \ + > "$guide_url_config" +cp /usr/share/nginx/html/registry/skill.md.template \ + /usr/share/nginx/html/registry/skill.md diff --git a/web/e2e/landing-quick-start-cli.spec.ts b/web/e2e/landing-quick-start-cli.spec.ts index 6b85f14b..1aa28ec8 100644 --- a/web/e2e/landing-quick-start-cli.spec.ts +++ b/web/e2e/landing-quick-start-cli.spec.ts @@ -50,15 +50,16 @@ test.describe('Landing access methods (Real API)', () => { expect(guideResponse.status()).toBe(200) const guide = await guideResponse.text() expect(guide).toContain('name: skillhub-cli') - expect(guide).toContain('removing the trailing `/registry/skill.md`') + expect(guide).toContain( + 'removing the trailing `/registry/skill.md` from the URL used to fetch this guide', + ) + expect(guide).not.toContain('${SKILLHUB_PUBLIC_BASE_URL}') expect(guideResponse.headers()['cache-control']).toContain('no-cache') - const hostileHostResponse = await page.request.get('/registry/skill.md', { - headers: { Host: 'attacker.example' }, - }) - expect(hostileHostResponse.status()).toBe(403) const extensionHostResponse = await page.request.get('/registry/skill.md', { headers: { Host: 'chrome-extension:evil;echo_injected' }, }) expect(extensionHostResponse.status()).toBe(400) + const templateResponse = await page.request.get('/registry/skill.md.template') + expect(templateResponse.status()).toBe(404) }) }) diff --git a/web/nginx.conf.template b/web/nginx.conf.template index 37cec962..7bf1c1ee 100644 --- a/web/nginx.conf.template +++ b/web/nginx.conf.template @@ -77,15 +77,16 @@ server { location = /registry/skill.md { default_type text/plain; include /etc/nginx/skillhub-guide-public-url*.conf; - sub_filter_types text/plain; - sub_filter_once off; - sub_filter '__SKILLHUB_PUBLIC_BASE_URL__' '$proxy_x_forwarded_proto://$http_host$skillhub_forwarded_prefix'; add_header Cache-Control "no-cache"; add_header Content-Disposition "inline"; add_header X-Content-Type-Options "nosniff"; try_files $uri =404; } + location = /registry/skill.md.template { + return 404; + } + location = /runtime-config.js { add_header Cache-Control "no-store"; try_files $uri =404; diff --git a/web/src/docs/skill.md.template b/web/src/docs/skill.md.template index 946dd213..af69443a 100644 --- a/web/src/docs/skill.md.template +++ b/web/src/docs/skill.md.template @@ -1,7 +1,7 @@ --- name: skillhub-cli description: Connect an Agent to a SkillHub registry and use the official SkillHub CLI to search, install, list, or explicitly upgrade SkillHub skills. Use when a user asks to connect SkillHub, install a SkillHub skill, or manage skills previously installed from SkillHub. -version: 2.0.0 +version: 2.0.1 license: Apache-2.0 --- @@ -30,16 +30,18 @@ First check whether the command on `PATH` is the expected CLI: skillhub version ``` -Use it only when the output is `SkillHub CLI `. A different result may be an unrelated command with the same name. +Use it only when the output is `SkillHub CLI `. If the command is missing, install the latest first-party CLI globally so future manual `skillhub` commands use this implementation: -When connecting this registry, install the latest first-party CLI globally so future manual `skillhub` commands use this implementation: - -```console +```bash npm install --global @astron-team/skillhub skillhub version ``` -If `skillhub version` still resolves to a known third-party launcher after installation, locate the exact command selected by the shell, remove that conflicting launcher only when it is owned by the current user, refresh command lookup, and run the global installation again. Do not retain or create an alias for the replaced command. Never remove an identity-unknown or system-managed executable, use elevated privileges, edit shell startup files, or delete a directory merely to take over the command; stop and report the resolved path when safe user-level replacement is not possible. +If `skillhub version` returns anything else, do not run the global installation yet because its package-manager shim could overwrite the existing launcher. Inspect the existing command without changing anything: resolve the exact command selected by the shell, follow symlinks to the final target, and identify its owner and installing package manager or package. Report those facts to the user. Do not infer identity from the command name or output alone. + +Only after the user separately confirms removal of that exact identified launcher may you use its package manager's supported uninstall command, refresh command lookup, and install the first-party CLI. Never unlink an executable directly, remove an identity-unknown or system-managed command, use elevated privileges, edit shell startup files, or delete a directory merely to take over the command. If the owner or package source cannot be proven, stop and give the user the resolved path and read-only findings. + +When the existing command already reports `SkillHub CLI `, connecting authorizes updating that verified first-party installation to the latest release with the same global npm command. Verify `skillhub version` afterward. Replacing the executable must not replace the other tool's data. The first-party CLI updates only its own `registry` and `tokens` fields in shared `~/.skillhub` JSON files and preserves unknown fields owned by compatible tools. Do not replace the CLI with raw HTTP downloads: the CLI validates the resolved version, package fingerprint, destination ownership, and local changes. Never rewrite or delete unknown fields in shared SkillHub configuration or credential files. @@ -59,7 +61,7 @@ Repository documentation may describe unreleased behavior. If neither live help - **Discover a Skill:** search this registry first. If it is unavailable or has no suitable result, report that outcome and ask before querying another registry. - **Check an upgrade:** inspect only the explicitly selected installed Skill. Never upgrade every installation implicitly. -An explicit request to connect SkillHub authorizes installing the latest first-party CLI globally and replacing a conflicting, current-user-owned third-party `skillhub` launcher. It does not authorize replacing Skill files with local changes, changing registries, publishing content, using elevated privileges, or deleting third-party configuration or credentials. +An explicit request to connect SkillHub authorizes installing the latest first-party CLI globally. It does not authorize removing another `skillhub` launcher, replacing Skill files with local changes, changing registries, publishing content, using elevated privileges, or deleting third-party configuration or credentials. Launcher removal requires the separate, exact confirmation described above. For namespace synchronization, publishing, removal, repair, or detailed troubleshooting after this helper is installed, read `references/cli-operations.md`. Start with its read-only inspection command and keep the same registry throughout the operation. diff --git a/web/src/i18n/landing-quick-start-locale.test.ts b/web/src/i18n/landing-quick-start-locale.test.ts index 9d5990e0..7b006d60 100644 --- a/web/src/i18n/landing-quick-start-locale.test.ts +++ b/web/src/i18n/landing-quick-start-locale.test.ts @@ -38,14 +38,16 @@ describe('landing quick start locales', () => { it('keeps the native CLI guide bound to the selected registry', () => { expect(skillGuideTemplate).toContain('name: skillhub-cli') - expect(skillGuideTemplate).toContain('version: 2.0.0') + expect(skillGuideTemplate).toContain('version: 2.0.1') expect(skillGuideTemplate).toContain('npm install --global @astron-team/skillhub') expect(skillGuideTemplate).not.toContain('@astron-team/skillhub@0.1.12') expect(skillGuideTemplate).toContain('the `registry` field in `~/.skillhub/config.json`') expect(skillGuideTemplate).toContain('`https://skill.xfyun.cn`') expect(skillGuideTemplate).not.toContain('${SKILLHUB_PUBLIC_BASE_URL}') - expect(skillGuideTemplate).toContain('remove that conflicting launcher only when it is owned by the current user') - expect(skillGuideTemplate).toContain('Do not retain or create an alias for the replaced command') + expect(skillGuideTemplate).toContain('separately confirms removal of that exact identified launcher') + expect(skillGuideTemplate).toContain('Never unlink an executable directly') + expect(skillGuideTemplate).toContain('do not run the global installation yet') + expect(skillGuideTemplate).toContain('does not authorize removing another `skillhub` launcher') expect(skillGuideTemplate).toContain('Treat `` below as a value to replace') expect(skillGuideTemplate).toContain([ 'skillhub install @global/skillhub-cli \\', diff --git a/web/vite.config.ts b/web/vite.config.ts index c647ad03..4b852cd7 100644 --- a/web/vite.config.ts +++ b/web/vite.config.ts @@ -13,6 +13,7 @@ const safeHostPattern = /^(?:[A-Za-z0-9.-]+|\[[0-9A-Fa-f:.]+\])(?::[0-9]{1,5})?$ function registryGuidePlugin(): Plugin { const basePrefix = basePath === '/' ? '' : basePath.slice(0, -1) const guidePath = `${basePrefix}/registry/skill.md` + const guideTemplatePath = `${basePrefix}/registry/skill.md.template` return { name: 'skillhub-cli-guide', @@ -24,11 +25,17 @@ function registryGuidePlugin(): Plugin { }) }, configureServer(server) { - // Install after Vite's built-in Host check so an untrusted Host can never - // be reflected into CLI commands. originalUrl survives SPA/base rewrites. + // Install after Vite's built-in Host check and reject malformed Host + // values consistently with the production route. originalUrl survives + // SPA/base rewrites. return () => { server.middlewares.use((request, response, next) => { const requestPath = new URL(request.originalUrl ?? request.url ?? '/', 'http://localhost').pathname + if (requestPath === guideTemplatePath) { + response.statusCode = 404 + response.end('Not Found') + return + } if (requestPath !== guidePath) { next() return @@ -41,12 +48,10 @@ function registryGuidePlugin(): Plugin { return } - const publicBaseUrl = `http://${host}${basePrefix}` - const guide = guideTemplate.replaceAll('${SKILLHUB_PUBLIC_BASE_URL}', publicBaseUrl) response.statusCode = 200 response.setHeader('Content-Type', 'text/markdown; charset=utf-8') response.setHeader('Cache-Control', 'no-cache, no-store, must-revalidate') - response.end(guide) + response.end(guideTemplate) }) } },