From ff3bc242cc8c51c7d7f5540c867e0472cb2da5a8 Mon Sep 17 00:00:00 2001 From: zayoka <141497128+zayokami@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:34:11 +0800 Subject: [PATCH] fix(Security): zip-slip on skill install, two credential leaks, and three panic paths (#5811) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(security): harden GrokBuild credential handling and Codex/Anthropic transforms against malformed input Three robustness/security fixes in the upstream v3.18.0 code, each with a regression test. 1) grok_config: remove the unconditional XAI_API_KEY fallback in extract_credentials(). Credentials now come only from an explicit inline api_key or the process env var named by env_key. Silently substituting a different account's key (when the declared env_key var is unset) could leak that key to whatever base_url the config points at. 2) deeplink/provider: merge_grokbuild_config no longer resolves env vars into a plaintext api_key on import. A deeplink is untrusted input; resolving+inlining would persist the victim's environment secret into the imported provider's config.toml and ship it to the link's declared base_url. env_key now stays an indirection (name), not a resolved secret. 3) proxy transforms: stop panicking on malformed upstream data. - transform_codex_anthropic::anthropic_sse_to_message_value: only store a `message`/`content_block` when it is an object; otherwise treat it as empty, so the later `["content"]`/`["text"]`/`["signature"]` index assignments can't panic on a scalar/array Value. - streaming_codex_anthropic::responses_sse_events_from_anthropic_message: bail out with a failed-event when the buffered body is a top-level JSON array/scalar (a gateway that ignores stream:true), instead of index-assigning into a non-object. - mcp/grokbuild sync: normalize a non-table `mcp_servers` before inserting, avoiding a toml_edit IndexMut panic on a user-edited config.toml. Panic findings verified with a minimal repro (serde_json index-assignment on a non-object Value aborts). cargo test --lib: 2183 passed / 0 failed. Co-Authored-By: Claude * fix(mcp): normalize a non-table `mcp_servers` in Codex config before insert Same class of bug as the GrokBuild MCP fix in the previous commit, but in the much more widely used Codex path. `sync_single_server_to_codex` guarded only with `contains_key("mcp_servers")`, so a user-edited `~/.codex/config.toml` where the key exists but is *not* a table (`mcp_servers = "x"` / `[]` / `42`) skipped the rebuild and then hit `doc["mcp_servers"][id] = …`, which panics in toml_edit's `IndexMut` (`.expect("index not found")`). The panic happens inside a Tauri command and unwinds across the FFI boundary; in the provider-switch flow it also fires after the DB/live write already committed, leaving a half-applied switch. Extract the normalize-then-insert step into `upsert_mcp_server_table()` so the previously panic-prone logic is unit-testable without touching the real `~/.codex/config.toml`, and normalize any non-table value to an empty table before inserting. Audited the sibling MCP writers while here: claude.rs / gemini.rs / hermes.rs / opencode.rs do not have this pattern (their `contains_key` uses are read-only checks), and the other index-assignments in codex.rs operate on freshly built `Table::new()` values, which are safe. Tests: 2 new regression tests (malformed non-table values normalize and insert; an existing valid table keeps its entries). cargo test --lib 2185 passed / 0 failed; cargo clippy --lib -D warnings clean. Co-Authored-By: Claude * fix(security): reject path-traversal entries when extracting a skill repo archive `SkillService::download_and_extract` built the output path from the raw ZIP entry name (`file.name()`), stripped only the leading `-/` component, and then `dest.join(relative_path)` → `fs::File::create`. A crafted entry such as `repo-main/../../../evil.sh` therefore escaped the destination directory (zip-slip): arbitrary file write outside the temp extraction dir. The archive is third-party controlled: it is downloaded from `https://github.com///archive/refs/heads/.zip`, and a skill repo (`owner`/`name`) can be added through an untrusted `ccswitch://` deeplink (`deeplink/skill.rs`), so the attacker fully controls the archive contents. Fix: resolve each entry through `zip::read::ZipFile::enclosed_name()`, which rejects `..` components and absolute paths, before stripping the archive's root directory and joining onto `dest`. Unsafe entries are skipped with a warning. This matches what the two sibling extractors in this codebase already did correctly (`extract_local_zip`, `webdav_sync/archive.rs`) — this call site was the one that had been missed. The extraction loop is split out into `extract_repo_archive()` so the guard is testable without network access. Verified the test actually catches the bug: with the guard reverted to the old `file.name()` behaviour the new test fails ("zip-slip entry must not escape dest (temp root)"); with the guard in place it passes. cargo test --lib: 2186 passed / 0 failed; cargo clippy --lib -D warnings clean. Co-Authored-By: Claude * fix(security): strip all credentials from the shared Gemini common-config snippet `extract_gemini_common_config` skipped only two hardcoded keys (`GOOGLE_GEMINI_BASE_URL`, `GEMINI_API_KEY`) and copied every other `env` entry into the shared snippet. But `GOOGLE_API_KEY` is a first-class Gemini credential — `provider.rs` resolves it via `first_non_empty(env, &["GEMINI_API_KEY", "GOOGLE_API_KEY"])` — so it was never stripped. That snippet is not inert: `apply_common_config_to_settings` (live.rs) deep-merges it into the `env` of *every other* Gemini provider that uses the common config, and the snippet is auto-extracted on startup, on import, and on switch. Net effect: account A's key gets written into provider B and sent to B's `GOOGLE_GEMINI_BASE_URL`, which may be a third-party relay. Anything else the user put in `env` (`GOOGLE_APPLICATION_CREDENTIALS`, a proxy `*_AUTH_TOKEN`, …) leaked the same way. Fix: also skip `Self::is_sensitive_config_key(key)`, reusing the pattern matcher the Claude extractor already relies on for exactly this reason (its comment notes a fixed enumeration "will always miss the next `*_API_KEY`"). `GOOGLE_API_KEY` matches its `_API_KEY` suffix rule. Shareable non-secret config (e.g. `GEMINI_TIMEOUT_MS`) is preserved. Verified the test catches the bug: run against the old two-key filter it fails with "credential GOOGLE_API_KEY must not leak into the shared Gemini snippet"; with the fix it passes. cargo test --lib: 2187 passed / 0 failed; cargo clippy --lib -D warnings clean. Co-Authored-By: Claude * fix(security): validate skill directory from backups and sync-imported DB rows The skill install pipeline sanitizes the install directory name, but two entry points bypassed it entirely: - restore_from_backup joined metadata.skill.directory (raw meta.json content) into the SSOT dir with no validation, allowing a crafted backup to copy attacker-controlled files outside the skills directory and to persist the poisoned value into the database. - Sync import (WebDAV/S3) loads the remote database dump verbatim, so a malicious or compromised sync snapshot could plant a skills row whose directory contains path traversal. Every later raw join then operated on attacker-controlled paths — uninstall/remove_from_app would remove_dir_all outside the managed dirs (arbitrary directory deletion), and sync_to_app_dir would write/symlink outside them. Add require_valid_directory() (built on the existing sanitize_install_name) and enforce it at restore_from_backup, sync_to_app_dir, remove_from_app, and uninstall. Regression tests cover all three sinks plus the metadata path; each was verified against the unguarded code first (disabling the guard makes the restore/uninstall tests complete the traversal write/delete, failing the assertions). Also fixes a pre-existing cargo fmt drift in mcp/grokbuild.rs. Co-Authored-By: Claude Fable 5 * fix(security): reject path separators in sanitize_install_name on all platforms The previous components()-based check was platform-dependent: on Linux/macOS a backslash is not a path separator, so "a\b" parsed as a single Normal component and was accepted as a valid install name — the same value becomes a nested path when synced or restored onto Windows. CI caught this on the Linux runner (the Windows runner passed). Reject both '/' and '\' explicitly so the validation is platform-independent; the components() check stays as the second layer for dot segments, roots, and prefixes. Co-Authored-By: Claude Fable 5 * fix(security): close the remaining skill-install attack chain The zip-slip guard added earlier only covered half the problem, and the repo coordinates that decide *which* archive gets downloaded were never validated at all. Together those two gaps let an attacker choose both the bytes and where they land. 1) zip-slip, second layer. `enclosed_name()` only guarantees the entry does not escape the *archive's own* root, and it does not normalise the path — `..` survives verbatim in the returned value. Stripping `root_name` afterwards spends one level of that depth budget, so `repo-main/../evil` still escaped `dest`. On Windows it is worse: `root_name` comes from `split('/')` and may contain backslashes, which Windows treats as separators, so one `strip_prefix` can eat N components. Re-check the *actual* relative path for `ParentDir` right before `dest.join()`. Verified by reverting the guard: the single-`..` case escapes without it. (The existing test used two `..`, which `enclosed_name()` rejects on its own — it covered the half that was already guarded.) 2) Repo coordinates are now validated. `download_repo` formats owner/name/ branch straight into `https://github.com/{owner}/{name}/archive/refs/heads/{branch}.zip`, and `deeplink/skill.rs` applied no validation whatsoever. URL parsing resolves dot segments, so a branch of `../../../releases/download/v1/evil` retargets the request at a *release asset* — arbitrary attacker-uploaded bytes. That is what made (1) reachable in practice: git itself will not let a tree contain `..`. Note the trigger is below "install": repos are enabled by default, so merely opening the Skills panel downloads and extracts. `validate_repo_ref` whitelists all three fields. Branch names legally contain `/` (`feature/x`), so the check is per segment rather than a blanket separator ban; an empty branch keeps its existing "use the default branch" sentinel meaning, same as `HEAD`. The main guard sits in `download_repo` — the single convergence point for all four call paths — because both `skill_repos` and `skills` can be overwritten wholesale by a sync snapshot, which no insert-time check can prevent. Entry-point checks (deeplink, `add_skill_repo`, agents lock) exist so bad values surface immediately instead of silently failing on every later panel open. `build_skill_doc_url` is covered too: it feeds `readme_url`, which the frontend hands to `openExternal`. 3) Regressions from the previous commits in this branch, both fixed: - `uninstall` ran the directory guard *before* `db.delete_skill`. That method has exactly one call site and is not exposed as a command, so a row with a dirty `directory` (pre-v3.11.0 installs, or anything `import_from_apps` still creates today — it has no sanitiser) became permanently undeletable from the UI. Now the guard skips the filesystem work but still deletes the row. - `sync_to_app` propagated with `?`, so one bad row aborted the entire app's skill sync — and that runs on provider switch. Now it warns and continues per entry. - `require_valid_directory` returned `sanitize_install_name`'s normalised value, which trims. A DB value of `" foo "` would then join as `"foo"` and miss the real directory. It now validates without rewriting. 4) Guard applied to the sinks the earlier commits missed: `migrate_storage` (rename + remove_dir_all), `update_skill` (delete + write attacker-named paths), `import_from_apps` (both a sink and the source of dirty values — `selection.directory` arrives raw over IPC), `resolve_uninstall_backup_source` (copies any directory into the backup area, which the UI then lists), `check_updates`, and `migrate_skills_to_ssot`. 5) Extraction limits. The archive bytes were fully attacker-controlled via (2), and this path had no ceiling of any kind, unlike `webdav_sync/archive.rs`. Added entry count, total extracted bytes, a symlink-target cap (zip 2.4.2's `make_reader` does not truncate at the declared `uncompressed_size`, so a symlink-flagged entry that inflates to gigabytes was read straight into memory), per-directory charging (a directory-only archive writes no content bytes but still consumes inodes), and a download-body cap (`response.bytes()` buffered the whole archive before any limit applied). Budget is charged on bytes actually read and written, never on sizes declared in the archive header. Symlink materialisation is charged to the same budget, and a target that contains the link itself (`dir/link -> ..`) is now rejected: the existing "must stay inside base" check passes for it, and the recursive copy then re-copies its own output every level until PATH_MAX. 6) Temp directories are now RAII. `download_repo` and `extract_local_zip` called `TempDir::keep()` immediately and relied on every exit path remembering `remove_dir_all`. Several did not — including `fetch_repo_skills`, the highest-frequency path of all. Returning the guard makes the leak unrepresentable at the call sites. * fix(config): make removals and edits survive user-authored config shapes The previous commits hardened the *write* side of the MCP tables against a non-table `mcp_servers`. The read/delete side kept using `as_table_mut`, which returns None for an inline table (`mcp_servers = { foo = {...} }` — valid TOML). Removal then silently did nothing: the UI reported success, the entry stayed in the file, and Codex loaded it again on next start. That is worse than the panic it mirrors, because users usually reach for the toggle precisely when an MCP server is misbehaving. Both Codex and GrokBuild now use `as_table_like_mut` on both sides. Normalisation is no longer silent either. Replacing a user's hand-written `mcp_servers = "x"` with an empty table destroys data, so all four sites that do it now log a warning first. `update_codex_toml_field` had the same asymmetry with a different symptom: when `model_providers` or `[model_providers.]` was an inline table, `as_table_mut` returned None and execution fell through to the "write a top-level field" fallback. The user's `base_url` edit landed at the wrong level, Codex never read it, and nothing reported a problem. `opencode_config` parses `~/.config/opencode/opencode.json` with json5 into a bare Value and never checked the root's shape. A user file of `[]` or a scalar made `set_provider`, `set_mcp_server` and `add_plugin` all panic on index-assignment, inside a Tauri command, unwinding across the FFI boundary. (A top-level `null` is fine — serde_json promotes it.) Rejecting a non-object root at the single read site fixes all three. Rejecting rather than rebuilding is deliberate: the file also holds the user's own `model` / `theme` settings, so a silent rebuild would delete them. Same treatment for the `provider` and `mcp` sections, whose non-object forms made writes silently no-op. * fix(proxy): recover a malformed Anthropic content_block as text Sanitising a non-object `content_block` to `{}` stops the panic but creates a quieter failure: the final Responses conversion matches on the block's `type` and silently drops anything it does not recognise, so a garbled block header turned into a `completed` response with empty output. The client saw the model say nothing, with no signal that data had been discarded. The replacement now carries `type: "text"`. The deltas that follow a malformed header are usually well-formed, so this recovers the common case; a tool-use block still yields nothing, exactly as before. A warning is logged when the substitution happens. The regression test now asserts through `anthropic_response_to_responses` rather than on the intermediate value — asserting on the intermediate alone passes while the client still receives an empty response. * fix(grokbuild): decouple base_url from credential resolution, reject env_key-only links `resolve_usage_credentials` called `extract_credentials(...).unwrap_or_default()`, so a missing credential blanked the base_url too — even though it is written right there in the config. Removing the `XAI_API_KEY` fallback in the earlier commit widened that considerably: a GUI process on macOS does not inherit the shell environment, so an `env_key` that resolves fine in a terminal resolves to nothing here. The result was a Base URL shown in the UI that differed from the one actually used, `{{baseUrl}}` expanding to empty in usage scripts (turning requests relative), and native balance queries reporting "API key is empty" while hiding the real cause. The two values are now resolved independently, via the existing `grok_config::extract_base_url`, matching how the Codex arm of the same function already works. A deeplink whose only credential is `env_key` is now rejected by name. It was already unimportable — `build_grokbuild_settings` has no `env_key` slot — but it failed with the generic "API key is required", which reads like a malformed link and invites the obvious "just carry the name over" fix. Carrying it over is exactly what must not happen: the forwarder and the usage query both resolve `env_key` at request time, so the victim's environment secret would still reach the link's `base_url`. Same leak, merely deferred. The message says so, and the test sets the probe variable so that restoring the resolution turns it red. * fix(security): scrub credentials already leaked into the Gemini shared snippet Fixing the extractor only prevents new contamination. A Gemini snippet is never re-extracted once it exists — startup auto-extract and post-import extraction both require `snippet.is_none()`, and the on-switch rewrite only covers Claude and Codex — so existing users keep injecting the leaked key into live config, the proxy upstream, and the new-provider form, where another account's key is plainly visible. Removing it from the snippet alone would make things worse. Merge and strip cancel out by *value equality*: on switch-away, `remove_common_config_from_settings` deletes the injected keys using the snippet as its only record of what to look for. Once the key is gone from the snippet, the residue left in live config is backfilled verbatim into the victim provider's `settings_config`, turning a transient leak into a permanent one. So the cleanup covers all four locations at once, and step order is itself a safety property: every fallible step runs before the irreversible one, and a failure returns an error so the next start retries from an intact state. Deletion is by key *and* value, never by key name alone, so a provider's own same-named key with a different value survives. The `~/.gemini/.env` edit preserves layout rather than round-tripping through `parse_env_file`/`serialize_env_file`: that pair drops comments, blank lines and unparseable lines, collapses duplicate keys and re-sorts the file. Acceptable when re-projecting everything, destructive for a targeted removal the user never asked for — in testing it reduced a six-line fixture to one line, taking the user's own key with it. An audit record is written before any provider row changes, listing key names and affected provider ids but *no values*: `settings` is not in `SYNC_SKIP_TABLES`, so it is uploaded by WebDAV/S3 sync, and these are precisely the credentials that must be destroyed. Keeping values would trade one deletion for a plaintext copy that spreads across devices, has no UI to reach it, and never expires. It is written only when absent, so a partially-applied earlier run cannot overwrite the original pre-cleanup state. Consequence, intentional: some Gemini providers will now report a missing API key and need one entered. That key was never theirs. (The victim's own value was already overwritten at merge time and cannot be recovered — worth noting in the release notes, along with a recommendation to rotate the leaked key.) The snippet row is deleted rather than set to `{}` when nothing shareable remains, and the `cleared` flag is deliberately not set — either would disable auto-extract permanently and prevent the user's legitimate shared config from ever coming back. The frontend snippet validator is aligned with the backend matcher. It had the same two hardcoded key names, so a hand-edited snippet could put `GOOGLE_API_KEY` straight back with no error. --------- Co-authored-by: Claude Co-authored-by: Jason --- src-tauri/src/codex_config.rs | 66 +- src-tauri/src/commands/skill.rs | 4 + src-tauri/src/deeplink/provider.rs | 137 +- src-tauri/src/deeplink/skill.rs | 15 +- src-tauri/src/gemini_config.rs | 66 +- src-tauri/src/grok_config.rs | 75 +- src-tauri/src/lib.rs | 11 + src-tauri/src/mcp/codex.rs | 175 +- src-tauri/src/mcp/grokbuild.rs | 44 +- src-tauri/src/opencode_config.rs | 110 +- src-tauri/src/provider.rs | 22 +- .../providers/streaming_codex_anthropic.rs | 24 + .../providers/transform_codex_anthropic.rs | 68 +- src-tauri/src/services/provider/mod.rs | 713 +++++++- src-tauri/src/services/skill.rs | 1590 +++++++++++++++-- .../forms/hooks/useGeminiCommonConfig.ts | 72 +- src/i18n/locales/en.json | 3 + src/i18n/locales/ja.json | 3 + src/i18n/locales/zh-TW.json | 3 + src/i18n/locales/zh.json | 3 + src/lib/errors/skillErrorParser.ts | 3 + 21 files changed, 2992 insertions(+), 215 deletions(-) diff --git a/src-tauri/src/codex_config.rs b/src-tauri/src/codex_config.rs index e1f9cd65d..61764425c 100644 --- a/src-tauri/src/codex_config.rs +++ b/src-tauri/src/codex_config.rs @@ -1908,25 +1908,53 @@ pub fn update_codex_toml_field(toml_str: &str, field: &str, value: &str) -> Resu if let Some(provider_key) = model_provider { // Ensure [model_providers] table exists - if doc.get("model_providers").is_none() { + // + // 用 as_table_like_mut 而非 as_table_mut：用户把配置写成 inline table + // （`model_providers = { foo = {...} }`，TOML 合法）时 as_table_mut + // 返回 None，会一路掉进下面的顶层 fallback——用户改的 base_url 被写到 + // 了错误层级且毫无提示。 + if doc + .get("model_providers") + .is_none_or(|item| item.as_table_like().is_none()) + { + // 键存在但不是表（`model_providers = 42`）时，下面这行会把用户 + // 手写的值替换掉。旧代码在这种形状下会掉进顶层 fallback 而不动 + // 它，所以归一化必须留痕——与 mcp/codex.rs、mcp/grokbuild.rs、 + // opencode_config.rs 的同款处理保持一致。 + if doc + .get("model_providers") + .is_some_and(|item| !item.is_none()) + { + log::warn!("config.toml 的 model_providers 不是表，已重置为空表"); + } doc["model_providers"] = toml_edit::table(); } - if let Some(model_providers) = doc["model_providers"].as_table_mut() { + if let Some(model_providers) = doc + .get_mut("model_providers") + .and_then(toml_edit::Item::as_table_like_mut) + { // Ensure [model_providers.] table exists if !model_providers.contains_key(&provider_key) { - model_providers[&provider_key] = toml_edit::table(); + model_providers.insert(&provider_key, toml_edit::table()); } - if let Some(provider_table) = model_providers[&provider_key].as_table_mut() { + if let Some(provider_table) = model_providers + .get_mut(&provider_key) + .and_then(toml_edit::Item::as_table_like_mut) + { if trimmed.is_empty() { provider_table.remove(field); } else { - provider_table[field] = toml_edit::value(trimmed); + provider_table.insert(field, toml_edit::value(trimmed)); } return Ok(doc.to_string()); } } + + log::warn!( + "config.toml 的 [model_providers.{provider_key}] 结构异常，{field} 改写为顶层字段" + ); } // Fallback: no model_provider or structure mismatch → top-level field @@ -2585,6 +2613,34 @@ model = "gpt-4" assert_eq!(base_url, "https://fallback.api/v1"); } + #[test] + fn base_url_writes_into_inline_table_provider_section() { + // inline table 是合法 TOML，但 as_table_mut() 对它返回 None。旧代码会因此 + // 掉进「写顶层字段」的 fallback：用户改的 base_url 落在错误层级， + // Codex 读不到，且界面毫无提示。 + let input = r#"model_provider = "any" +model_providers = { any = { name = "any", base_url = "https://old.api/v1", wire_api = "responses" } } +"#; + + let result = update_codex_toml_field(input, "base_url", "https://new.api/v1").unwrap(); + let parsed: toml::Value = toml::from_str(&result).unwrap(); + + assert_eq!( + parsed["model_providers"]["any"]["base_url"].as_str(), + Some("https://new.api/v1"), + "must update the provider section, not a top-level field" + ); + assert!( + parsed.get("base_url").is_none(), + "must not leak a top-level base_url fallback" + ); + assert_eq!( + parsed["model_providers"]["any"]["wire_api"].as_str(), + Some("responses"), + "sibling fields must survive" + ); + } + #[test] fn clearing_base_url_removes_only_from_correct_section() { let input = r#"model_provider = "any" diff --git a/src-tauri/src/commands/skill.rs b/src-tauri/src/commands/skill.rs index feb0bfc00..6ac90ac54 100644 --- a/src-tauri/src/commands/skill.rs +++ b/src-tauri/src/commands/skill.rs @@ -301,6 +301,10 @@ pub fn get_skill_repos(app_state: State<'_, AppState>) -> Result, /// 添加技能仓库 #[tauri::command] pub fn add_skill_repo(repo: SkillRepo, app_state: State<'_, AppState>) -> Result { + // 整个结构体由前端反序列化而来，owner/name/branch 会被拼进归档下载 URL。 + // 主防线在 download_repo，这里让非法值当场报错而不是沉淀进表。 + SkillService::validate_repo_ref(&repo.owner, &repo.name, &repo.branch) + .map_err(|e| e.to_string())?; app_state .db .save_skill_repo(&repo) diff --git a/src-tauri/src/deeplink/provider.rs b/src-tauri/src/deeplink/provider.rs index 9de6076bc..5febfda3f 100644 --- a/src-tauri/src/deeplink/provider.rs +++ b/src-tauri/src/deeplink/provider.rs @@ -832,9 +832,34 @@ fn merge_grokbuild_config( .as_ref() .is_none_or(|value| value.is_empty()) { - request.api_key = model.api_key.or_else(|| { - crate::grok_config::extract_credentials(&config_toml).map(|(_, api_key)| api_key) - }); + // Only inline an explicitly-declared `api_key`. Do NOT resolve `env_key` + // (or any process env var) into a plaintext value here: a deeplink is + // untrusted input, and resolving+inlining would silently persist the + // victim's environment secret into the imported provider's config.toml + // and ship it to whatever `base_url` the link declares. `env_key` is an + // indirection that must stay a name, not a resolved secret, on import. + request.api_key = model.api_key; + + // An `env_key`-only link is not importable at all, and saying so beats + // falling through to the generic "API key is required" (which reads like + // a malformed link and invites a "just carry the name over" fix). + // Carrying the name over is exactly what must not happen: the forwarder + // and the usage query both resolve `env_key` at request time, so the + // victim's environment secret would still reach the link's `base_url` + // — the same leak, merely deferred. + if request + .api_key + .as_ref() + .is_none_or(|value| value.is_empty()) + && model.env_key.is_some() + { + return Err(AppError::InvalidInput( + "This link supplies its API key indirectly through `env_key`, which cannot be \ + imported from an untrusted link. Add the provider manually and enter the key \ + yourself." + .to_string(), + )); + } } if request .endpoint @@ -929,6 +954,112 @@ mod tests { } } + /// deeplink 同时声明 `api_key` 与 `env_key` 时，导入结果只保留用户可见的 + /// `api_key`：既不能带上解析后的明文环境变量，也不能把 `env_key` 这个间接 + /// 引用本身写进供应商配置。 + /// + /// 后者容易被误当成「应该补上的功能」，但恰恰不能补：deeplink 是不可信输入， + /// 攻击者可以让 `env_key` 指向 `XAI_API_KEY`、`base_url` 指向自己的服务器。 + /// 密钥虽然导入时没落盘，却会在转发/用量查询调用 `extract_credentials` 时 + /// 被现场解析并发给攻击者——等于把已修掉的泄露换成延迟触发的版本。 + /// 手工建供应商的表单路径不受影响，那里的 env_key 是用户自己输入的。 + #[test] + #[serial_test::serial] + fn grokbuild_deeplink_never_carries_env_key_or_resolved_secret() { + let original = std::env::var_os("CC_SWITCH_DEEPLINK_ENV_PROBE"); + std::env::set_var("CC_SWITCH_DEEPLINK_ENV_PROBE", "secret-must-not-leak"); + + let mut request = DeepLinkImportRequest { + resource: "provider".to_string(), + app: Some("grokbuild".to_string()), + name: Some("Attacker".to_string()), + ..Default::default() + }; + let config = serde_json::json!({ + "config": concat!( + "[models]\ndefault = \"grok-env\"\n\n", + "[model.\"grok-env\"]\nmodel = \"grok-4.5\"\n", + "base_url = \"https://attacker.example/v1\"\nname = \"Attacker\"\n", + "api_key = \"sk-declared-by-link\"\n", + "env_key = \"CC_SWITCH_DEEPLINK_ENV_PROBE\"\n", + "api_backend = \"responses\"\ncontext_window = 500000\n", + ) + }); + + merge_grokbuild_config(&mut request, &config).expect("merge should succeed"); + let settings = build_grokbuild_settings(&request); + let rendered = settings["config"].as_str().expect("config string"); + + assert!( + !rendered.contains("secret-must-not-leak"), + "the environment secret must never be inlined: {rendered}" + ); + assert!( + !rendered.contains("env_key"), + "the env_key indirection must not be carried over from an untrusted link: {rendered}" + ); + assert!( + rendered.contains("api_key = \"sk-declared-by-link\""), + "only the explicitly declared api_key should survive: {rendered}" + ); + + match original { + Some(value) => std::env::set_var("CC_SWITCH_DEEPLINK_ENV_PROBE", value), + None => std::env::remove_var("CC_SWITCH_DEEPLINK_ENV_PROBE"), + } + } + + /// `env_key` 独苗的链接必须在**公开入口**上被明确拒绝。 + /// + /// 这条走 `parse_and_merge_config` 而不是内部 helper：真实失败路径在这里， + /// 而且报错必须指名 `env_key`——否则用户只看到泛化的 "API key is required"， + /// 读起来像链接坏了，下一步就会有人「顺手把 env_key 透传过去」，把泄露改成 + /// 延迟触发的版本。 + #[test] + #[serial_test::serial] + fn grokbuild_env_key_only_link_is_rejected_at_the_public_entry() { + use base64::prelude::*; + + // 探针变量必须**真的设上**。若它在环境里不存在，旧代码的 + // `extract_credentials` 回退也解析不出东西，api_key 同样留空、同样触发 + // 拒绝——测试就退化成只覆盖"没有 key 时报错"，对"不得解析环境变量"这条 + // 真正的安全属性零覆盖。设上之后，一旦有人恢复回退解析，api_key 会变成 + // 非空、拒绝不再触发，这条断言立刻变红。 + let original = std::env::var_os("CC_SWITCH_DEEPLINK_ENV_PROBE"); + std::env::set_var("CC_SWITCH_DEEPLINK_ENV_PROBE", "secret-must-not-leak"); + + let config_toml = concat!( + "[models]\ndefault = \"grok-env\"\n\n", + "[model.\"grok-env\"]\nmodel = \"grok-4.5\"\n", + "base_url = \"https://attacker.example/v1\"\nname = \"Attacker\"\n", + "env_key = \"CC_SWITCH_DEEPLINK_ENV_PROBE\"\n", + "api_backend = \"responses\"\ncontext_window = 500000\n", + ); + let request = DeepLinkImportRequest { + resource: "provider".to_string(), + app: Some("grokbuild".to_string()), + name: Some("Attacker".to_string()), + config: Some(BASE64_STANDARD.encode(config_toml)), + config_format: Some("toml".to_string()), + ..Default::default() + }; + + let result = parse_and_merge_config(&request); + + match original { + Some(value) => std::env::set_var("CC_SWITCH_DEEPLINK_ENV_PROBE", value), + None => std::env::remove_var("CC_SWITCH_DEEPLINK_ENV_PROBE"), + } + + let err = result.expect_err( + "an env_key-only link must not be importable, and its env var must never be resolved", + ); + assert!( + err.to_string().contains("env_key"), + "the rejection must name env_key so it is not mistaken for a malformed link: {err}" + ); + } + #[test] fn build_hermes_settings_emits_snake_case() { let settings = build_hermes_settings(&hermes_request()); diff --git a/src-tauri/src/deeplink/skill.rs b/src-tauri/src/deeplink/skill.rs index d4369eaf3..a5a669618 100644 --- a/src-tauri/src/deeplink/skill.rs +++ b/src-tauri/src/deeplink/skill.rs @@ -33,12 +33,25 @@ pub fn import_skill_from_deeplink( } let owner = parts[0].to_string(); let name = parts[1].to_string(); + let branch = request.branch.unwrap_or_else(|| "main".to_string()); + + // deeplink 是不可信输入，且 branch 会被拼进归档下载 URL：URL 解析会消解点段， + // `../../../releases/download/v1/evil` 之类会把落点改写成攻击者可上传的 + // release asset。主防线在 SkillService::download_repo，这里是入库前的纵深拦截， + // 让用户当场看到错误而不是让脏数据沉淀进 skill_repos 表。 + crate::services::skill::SkillService::validate_repo_ref(&owner, &name, &branch).map_err( + |_| { + AppError::InvalidInput(format!( + "Invalid skill repository reference: '{owner}/{name}' branch '{branch}'" + )) + }, + )?; // Create SkillRepo let repo = SkillRepo { owner: owner.clone(), name: name.clone(), - branch: request.branch.unwrap_or_else(|| "main".to_string()), + branch, enabled: request.enabled.unwrap_or(true), }; diff --git a/src-tauri/src/gemini_config.rs b/src-tauri/src/gemini_config.rs index 90fa28b2e..fdea2da5a 100644 --- a/src-tauri/src/gemini_config.rs +++ b/src-tauri/src/gemini_config.rs @@ -152,8 +152,71 @@ pub fn read_gemini_env() -> Result, AppError> { Ok(parse_env_file(&content)) } +/// 从 .env 原文中按「键名 + 值」双匹配删除若干行，其余内容逐字保留 +/// +/// 不走 `parse_env_file` → `serialize_env_file` 的往返：那对函数会丢掉注释、空行、 +/// 无法识别的行和重复定义，并按键名重排整个文件。全量投影时这无所谓（本来就要重写 +/// 整份），但用来做**定向**清理就等于顺手把用户手写的东西一起删了。 +/// +/// 按值匹配而非按键名：只清掉扩散出去的那一份，用户自己写的同名不同值的行保留。 +/// 同一个键有重复定义时也只删命中的那条，被它遮住的上一条会重新生效——这正是想要的 +/// 结果，因为遮住它的恰恰是泄漏值。 +/// +/// 返回 `None` 表示没有任何一行命中，调用方据此跳过写盘。 +pub fn remove_env_entries_preserving_layout( + content: &str, + doomed: &HashMap, +) -> Option { + let mut removed = false; + let mut kept: Vec<&str> = Vec::new(); + + for line in content.split('\n') { + let trimmed = line.trim(); + let hit = !trimmed.is_empty() + && !trimmed.starts_with('#') + && trimmed.split_once('=').is_some_and(|(key, value)| { + doomed + .get(key.trim()) + .is_some_and(|doomed_value| doomed_value == value.trim()) + }); + + if hit { + removed = true; + } else { + kept.push(line); + } + } + + removed.then(|| kept.join("\n")) +} + +/// 从 `~/.gemini/.env` 中定向删除「键=值」完全匹配的行，返回是否真的改了文件 +pub fn remove_gemini_env_entries(doomed: &HashMap) -> Result { + let path = get_gemini_env_path(); + if !path.exists() { + return Ok(false); + } + + let content = fs::read_to_string(&path).map_err(|e| AppError::io(&path, e))?; + match remove_env_entries_preserving_layout(&content, doomed) { + Some(cleaned) => { + write_gemini_env_text_atomic(&cleaned)?; + Ok(true) + } + None => Ok(false), + } +} + /// 写入 Gemini .env 文件（原子操作） pub fn write_gemini_env_atomic(map: &HashMap) -> Result<(), AppError> { + write_gemini_env_text_atomic(&serialize_env_file(map)) +} + +/// 写入 Gemini .env 文件（原子操作，内容逐字落盘） +/// +/// 与 `write_gemini_env_atomic` 共用目录/文件权限处理，区别只在于内容不经 +/// `serialize_env_file` 归一化——供保序的定向删除使用。 +pub fn write_gemini_env_text_atomic(content: &str) -> Result<(), AppError> { let path = get_gemini_env_path(); // 确保目录存在 @@ -172,8 +235,7 @@ pub fn write_gemini_env_atomic(map: &HashMap) -> Result<(), AppE } } - let content = serialize_env_file(map); - write_text_file(&path, &content)?; + write_text_file(&path, content)?; // 设置文件权限为 600（仅所有者可读写） #[cfg(unix)] diff --git a/src-tauri/src/grok_config.rs b/src-tauri/src/grok_config.rs index f0ab93864..fa1dcad65 100644 --- a/src-tauri/src/grok_config.rs +++ b/src-tauri/src/grok_config.rs @@ -209,22 +209,23 @@ pub fn extract_model_config(config_toml: &str) -> Option { pub fn extract_credentials(config_toml: &str) -> Option<(String, String)> { let config = extract_model_config(config_toml)?; - let api_key = config - .api_key - .or_else(|| { - config - .env_key - .as_deref() - .and_then(|key| std::env::var(key).ok()) - .map(|value| value.trim().to_string()) - .filter(|value| !value.is_empty()) - }) - .or_else(|| { - std::env::var("XAI_API_KEY") - .ok() - .map(|value| value.trim().to_string()) - .filter(|value| !value.is_empty()) - })?; + // Credentials only come from two explicit, config-declared sources: + // 1. an inline `api_key`, or + // 2. the process env var named by `env_key`. + // + // Deliberately NO unconditional fallback to `XAI_API_KEY`: silently + // substituting a different account's key (when the declared `env_key` var is + // unset) would leak that key to whatever `base_url` this config points at. + // An unset/missing declared credential must surface as "no credential" + // (None) so callers can fail loudly rather than transmit the wrong secret. + let api_key = config.api_key.or_else(|| { + config + .env_key + .as_deref() + .and_then(|key| std::env::var(key).ok()) + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()) + })?; Some((config.base_url, api_key)) } @@ -540,6 +541,48 @@ context_window = 500000 } } + /// 构造一个 `env_key` 指向未设置环境变量的 config——这是"声明了间接引用但 + /// 该变量不存在"的场景，修复前会静默兜底到 `XAI_API_KEY`。 + fn env_key_unset_config() -> &'static str { + r#"[models] +default = "grok-env" + +[model."grok-env"] +model = "grok-4.5" +base_url = "https://attacker.example/v1" +name = "Attacker Env" +env_key = "GROK_TEST_DEFINITELY_UNSET_VAR" +api_backend = "responses" +context_window = 500000 +"# + } + + #[test] + #[serial] + fn does_not_fall_back_to_xai_api_key_when_declared_env_key_is_unset() { + // 即使进程里恰好设了 XAI_API_KEY，也不能被静默借用到别的 base_url 上。 + let original_xai = std::env::var_os("XAI_API_KEY"); + let original_unset = std::env::var_os("GROK_TEST_DEFINITELY_UNSET_VAR"); + std::env::set_var("XAI_API_KEY", "xai-secret-should-not-leak"); + std::env::remove_var("GROK_TEST_DEFINITELY_UNSET_VAR"); + + let credentials = extract_credentials(env_key_unset_config()); + + assert!( + credentials.is_none(), + "declared env_key unset must yield None, never a borrowed XAI_API_KEY; got {credentials:?}" + ); + + match original_xai { + Some(value) => std::env::set_var("XAI_API_KEY", value), + None => std::env::remove_var("XAI_API_KEY"), + } + match original_unset { + Some(value) => std::env::set_var("GROK_TEST_DEFINITELY_UNSET_VAR", value), + None => std::env::remove_var("GROK_TEST_DEFINITELY_UNSET_VAR"), + } + } + #[test] fn strips_projected_mcp_servers_without_touching_model_config() { let mut settings = json!({ diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index bdf5b08b3..13b3b7ba8 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1184,6 +1184,17 @@ pub fn run() { } } + // 必须排在 auto-extract 之前：先把历史泄漏进 Gemini 共享片段的凭据 + // 清干净，否则紧接着的提取会基于被污染的 live 再写一遍。 + if let Err(e) = + crate::services::provider::ProviderService::scrub_leaked_gemini_common_config( + &state, + ) + .await + { + log::warn!("清理 Gemini 通用配置泄漏凭据失败: {e}"); + } + initialize_common_config_snippets(&state); // 检查 settings 表中的代理状态，自动恢复代理服务 diff --git a/src-tauri/src/mcp/codex.rs b/src-tauri/src/mcp/codex.rs index 7b20dcdbd..b6d6f61be 100644 --- a/src-tauri/src/mcp/codex.rs +++ b/src-tauri/src/mcp/codex.rs @@ -348,6 +348,74 @@ pub fn sync_enabled_to_codex(config: &MultiAppConfig) -> Result<(), AppError> { /// 将单个 MCP 服务器同步到 Codex live 配置 /// 始终使用 Codex 官方格式 [mcp_servers]，并清理可能存在的错误格式 [mcp.servers] +/// 把单个 MCP server 表写入 `[mcp_servers]`，并保证该键是「表」。 +/// +/// `~/.codex/config.toml` 是用户可手改的：若 `mcp_servers` 存在但不是表 +/// （如 `mcp_servers = "x"` / `[]`），仅判 `contains_key` 会跳过重建，随后的 +/// `doc["mcp_servers"][id] = …` 会触发 toml_edit 的 `IndexMut` panic +/// （panic 发生在 Tauri command 内、跨 FFI 展开）。这里统一归一化后再插入。 +fn upsert_mcp_server_table( + doc: &mut toml_edit::DocumentMut, + id: &str, + table: toml_edit::Table, +) -> Result<(), AppError> { + if doc + .get_mut("mcp_servers") + .and_then(toml_edit::Item::as_table_like_mut) + .is_none() + { + // 键存在但不是表时，归一化会丢掉用户手写的那个值——必须留痕， + // 否则用户只会看到自己的改动凭空消失。 + if doc.get("mcp_servers").is_some_and(|item| !item.is_none()) { + log::warn!("config.toml 的 mcp_servers 不是表，已重置为空表"); + } + doc["mcp_servers"] = toml_edit::table(); + } + let servers = doc + .get_mut("mcp_servers") + .and_then(toml_edit::Item::as_table_like_mut) + .ok_or_else(|| AppError::McpValidation("config.toml 的 mcp_servers 不是表".to_string()))?; + servers.insert(id, toml_edit::Item::Table(table)); + Ok(()) +} + +/// 从 `[mcp_servers]`（以及历史错误格式 `[mcp.servers]`）中删除单个 MCP server。 +/// +/// 与 `upsert_mcp_server_table` 对称地使用 `as_table_like_mut`：用户若把配置写成 +/// inline table（`mcp_servers = { foo = {...} }`，TOML 合法），`as_table_mut` 会返回 +/// None 导致删除**静默失效**——界面提示已移除，条目却还在文件里，Codex 下次启动照样 +/// 加载。这比 panic 更隐蔽，因为用户往往正是发现某个 MCP 有问题才来关它的。 +/// +/// 与写入分离成纯 doc 级函数，使守卫可脱离真实 `~/.codex/config.toml` 单测。 +fn remove_mcp_server_from_doc(doc: &mut toml_edit::DocumentMut, id: &str) { + if let Some(item) = doc.get_mut("mcp_servers") { + // `Item::None` 是 toml_edit 的占位形态，不是用户写下的值——对它告警是噪音。 + // 必须在取可变借用之前算出来。 + let user_authored = !item.is_none(); + match item.as_table_like_mut() { + Some(mcp_servers) => { + mcp_servers.remove(id); + } + None if user_authored => { + log::warn!("config.toml 的 mcp_servers 不是表，无法删除服务器 '{id}'"); + } + None => {} + } + } + + // 同时清理可能存在于错误位置的数据：[mcp.servers]（如果存在） + if let Some(mcp_table) = doc.get_mut("mcp").and_then(|t| t.as_table_like_mut()) { + if let Some(servers) = mcp_table + .get_mut("servers") + .and_then(|s| s.as_table_like_mut()) + { + if servers.remove(id).is_some() { + log::warn!("从错误的 MCP 格式 [mcp.servers] 中清理了服务器 '{id}'"); + } + } + } +} + pub fn sync_single_server_to_codex( _config: &MultiAppConfig, id: &str, @@ -356,7 +424,6 @@ pub fn sync_single_server_to_codex( if !should_sync_codex_mcp() { return Ok(()); } - use toml_edit::Item; // 读取现有的 config.toml let config_path = crate::codex_config::get_codex_config_path(); @@ -383,16 +450,9 @@ pub fn sync_single_server_to_codex( } } - // 确保 [mcp_servers] 表存在 - if !doc.contains_key("mcp_servers") { - doc["mcp_servers"] = toml_edit::table(); - } - // 将 JSON 服务器规范转换为 TOML 表 let toml_table = json_server_to_toml_table(server_spec)?; - - // 使用唯一正确的格式：[mcp_servers] - doc["mcp_servers"][id] = Item::Table(toml_table); + upsert_mcp_server_table(&mut doc, id, toml_table)?; // 写回文件 let new_text = doc.to_string(); @@ -425,19 +485,7 @@ pub fn remove_server_from_codex(id: &str) -> Result<(), AppError> { } }; - // 从正确的位置删除：[mcp_servers] - if let Some(mcp_servers) = doc.get_mut("mcp_servers").and_then(|s| s.as_table_mut()) { - mcp_servers.remove(id); - } - - // 同时清理可能存在于错误位置的数据：[mcp.servers]（如果存在） - if let Some(mcp_table) = doc.get_mut("mcp").and_then(|t| t.as_table_mut()) { - if let Some(servers) = mcp_table.get_mut("servers").and_then(|s| s.as_table_mut()) { - if servers.remove(id).is_some() { - log::warn!("从错误的 MCP 格式 [mcp.servers] 中清理了服务器 '{id}'"); - } - } - } + remove_mcp_server_from_doc(&mut doc, id); // 写回文件 let new_text = doc.to_string(); @@ -683,6 +731,89 @@ pub(super) fn json_server_to_toml_table(spec: &Value) -> Result() + .expect("fixture parses"); + let table = json_server_to_toml_table(&json!({ + "type": "stdio", + "command": "npx" + })) + .expect("server table"); + + upsert_mcp_server_table(&mut doc, "echo", table) + .unwrap_or_else(|e| panic!("upsert must not fail for {malformed:?}: {e}")); + + let servers = doc + .get("mcp_servers") + .and_then(|item| item.as_table_like()) + .unwrap_or_else(|| panic!("mcp_servers must be normalized to a table")); + assert!(servers.contains_key("echo")); + } + } + + #[test] + fn upsert_preserves_existing_servers_in_a_valid_table() { + let mut doc = "[mcp_servers.keep]\ncommand = \"keep\"\n" + .parse::() + .expect("fixture parses"); + let table = json_server_to_toml_table(&json!({ + "type": "stdio", + "command": "npx" + })) + .expect("server table"); + + upsert_mcp_server_table(&mut doc, "added", table).expect("upsert"); + + let servers = doc + .get("mcp_servers") + .and_then(|item| item.as_table_like()) + .expect("table"); + assert!(servers.contains_key("keep"), "existing server must survive"); + assert!(servers.contains_key("added")); + } + + #[test] + fn remove_deletes_from_inline_table_form_too() { + // inline table 是合法 TOML，但 as_table_mut() 对它返回 None——用它做守卫 + // 会让删除静默失效：界面说移除成功，条目却还在，Codex 下次启动照样加载。 + let mut doc = "mcp_servers = { drop = { command = \"x\" }, keep = { command = \"y\" } }\n" + .parse::() + .expect("fixture parses"); + + remove_mcp_server_from_doc(&mut doc, "drop"); + + let servers = doc + .get("mcp_servers") + .and_then(|item| item.as_table_like()) + .expect("mcp_servers must still be table-like"); + assert!( + !servers.contains_key("drop"), + "removal must work on the inline-table form" + ); + assert!(servers.contains_key("keep"), "siblings must survive"); + } + + #[test] + fn remove_is_a_noop_on_non_table_mcp_servers() { + // 既不能 panic，也不能把用户手写的值悄悄抹掉 + let mut doc = "mcp_servers = 42\n" + .parse::() + .expect("fixture parses"); + + remove_mcp_server_from_doc(&mut doc, "whatever"); + + assert_eq!(doc.to_string(), "mcp_servers = 42\n"); + } + #[test] fn http_headers_are_only_written_to_codex_http_headers() { let table = json_server_to_toml_table(&json!({ diff --git a/src-tauri/src/mcp/grokbuild.rs b/src-tauri/src/mcp/grokbuild.rs index 0cf0664e5..32bb04922 100644 --- a/src-tauri/src/mcp/grokbuild.rs +++ b/src-tauri/src/mcp/grokbuild.rs @@ -148,10 +148,30 @@ pub fn sync_single_server_to_grokbuild( AppError::McpValidation(format!("解析 Grok Build config.toml 失败: {e}")) })? }; - if !doc.contains_key("mcp_servers") { + // 若 mcp_servers 缺失或存在但不是 table（如 `mcp_servers = "x"` / `[]`）， + // 用空 table 归一化，避免后续 `doc["mcp_servers"][id] = …` 对非 table 索引 + // 触发 toml_edit 的 IndexMut panic。用户手写的 config.toml 不可信。 + // 判定走不可变的 `as_table_like`：借可变引用只为判空，会逼着下面再 get_mut 一次。 + if doc + .get("mcp_servers") + .is_none_or(|item| item.as_table_like().is_none()) + { + // 归一化会丢掉用户手写的那个非表值，必须留痕。 + if doc.get("mcp_servers").is_some_and(|item| !item.is_none()) { + log::warn!("Grok Build config.toml 的 mcp_servers 不是表，已重置为空表"); + } doc["mcp_servers"] = toml_edit::table(); } - doc["mcp_servers"][id] = Item::Table(json_server_to_grokbuild_toml_table(server_spec)?); + let servers = doc + .get_mut("mcp_servers") + .and_then(toml_edit::Item::as_table_like_mut) + .ok_or_else(|| { + AppError::McpValidation("Grok Build config.toml 的 mcp_servers 不是表".to_string()) + })?; + servers.insert( + id, + Item::Table(json_server_to_grokbuild_toml_table(server_spec)?), + ); crate::config::write_text_file(&path, &doc.to_string()) } @@ -171,11 +191,21 @@ pub fn remove_server_from_grokbuild(id: &str) -> Result<(), AppError> { return Ok(()); } }; - if let Some(servers) = doc - .get_mut("mcp_servers") - .and_then(toml_edit::Item::as_table_mut) - { - servers.remove(id); + // 与写入侧对称使用 as_table_like_mut：inline table 形态下 as_table_mut 返回 + // None，删除会静默失效——界面显示已移除，Grok Build 下次启动仍会加载它。 + if let Some(item) = doc.get_mut("mcp_servers") { + // `Item::None` 是 toml_edit 的占位形态，不是用户写下的值——对它告警是噪音。 + // 必须在取可变借用之前算出来。 + let user_authored = !item.is_none(); + match item.as_table_like_mut() { + Some(servers) => { + servers.remove(id); + } + None if user_authored => { + log::warn!("Grok Build config.toml 的 mcp_servers 不是表，无法删除服务器 '{id}'"); + } + None => {} + } } crate::config::write_text_file(&path, &doc.to_string()) } diff --git a/src-tauri/src/opencode_config.rs b/src-tauri/src/opencode_config.rs index 437c2a980..ce5a255c1 100644 --- a/src-tauri/src/opencode_config.rs +++ b/src-tauri/src/opencode_config.rs @@ -95,12 +95,27 @@ pub fn read_opencode_config() -> Result { } let content = std::fs::read_to_string(&path).map_err(|e| AppError::io(&path, e))?; - json5::from_str(&content).map_err(|e| { + let value: Value = json5::from_str(&content).map_err(|e| { AppError::Config(format!( "Failed to parse OpenCode config: {}: {e}", path.display() )) - }) + })?; + + // 根节点必须是对象：下游 set_provider / set_mcp_server / add_plugin 都对它做 + // `config["key"] = …` 索引赋值，而 serde_json 只把 Null 自动升级成对象， + // 数组或标量会直接 panic（panic 发生在 Tauri command 内、跨 FFI 展开）。 + // + // 这里选择报错而不是重建根节点：opencode.json 里还有 model / theme 等用户自有 + // 配置，静默重建等于删掉它们。让用户自己修文件，与 read_claude_live 的做法一致。 + if !value.is_object() { + return Err(AppError::Config(format!( + "OpenCode 配置文件根节点必须是 JSON 对象: {}", + path.display() + ))); + } + + Ok(value) } pub fn write_opencode_config(config: &Value) -> Result<(), AppError> { @@ -123,7 +138,13 @@ pub fn get_providers() -> Result, AppError> { pub fn set_provider(id: &str, config: Value) -> Result<(), AppError> { let mut full_config = read_opencode_config()?; - if full_config.get("provider").is_none() { + // 判空要连「存在但不是对象」一起算：否则下面 as_object_mut 拿不到， + // 写入会静默失效——界面显示添加成功而文件里没有。provider 段是 cc-switch + // 的投影区，归一化不会碰用户自有的 model / theme 等顶层配置。 + if !full_config.get("provider").is_some_and(Value::is_object) { + if full_config.get("provider").is_some() { + log::warn!("opencode.json 的 provider 不是对象，已重置为空对象"); + } full_config["provider"] = json!({}); } @@ -142,6 +163,8 @@ pub fn remove_provider(id: &str) -> Result<(), AppError> { if let Some(providers) = config.get_mut("provider").and_then(|v| v.as_object_mut()) { providers.remove(id); + } else if config.get("provider").is_some() { + log::warn!("opencode.json 的 provider 不是对象，无法删除供应商 '{id}'"); } write_opencode_config(&config) @@ -182,7 +205,10 @@ pub fn get_mcp_servers() -> Result, AppError> { pub fn set_mcp_server(id: &str, config: Value) -> Result<(), AppError> { let mut full_config = read_opencode_config()?; - if full_config.get("mcp").is_none() { + if !full_config.get("mcp").is_some_and(Value::is_object) { + if full_config.get("mcp").is_some() { + log::warn!("opencode.json 的 mcp 不是对象，已重置为空对象"); + } full_config["mcp"] = json!({}); } @@ -198,6 +224,8 @@ pub fn remove_mcp_server(id: &str) -> Result<(), AppError> { if let Some(mcp) = config.get_mut("mcp").and_then(|v| v.as_object_mut()) { mcp.remove(id); + } else if config.get("mcp").is_some() { + log::warn!("opencode.json 的 mcp 不是对象，无法删除服务器 '{id}'"); } write_opencode_config(&config) @@ -265,3 +293,77 @@ pub fn remove_plugins_by_prefixes(prefixes: &[&str]) -> Result<(), AppError> { write_opencode_config(&config) } + +#[cfg(test)] +mod tests { + use super::*; + + struct TestHomeGuard(Option); + impl TestHomeGuard { + fn set(home: &std::path::Path) -> Self { + let guard = Self(std::env::var_os("CC_SWITCH_TEST_HOME")); + std::env::set_var("CC_SWITCH_TEST_HOME", home); + guard + } + } + impl Drop for TestHomeGuard { + fn drop(&mut self) { + match self.0.take() { + Some(value) => std::env::set_var("CC_SWITCH_TEST_HOME", value), + None => std::env::remove_var("CC_SWITCH_TEST_HOME"), + } + } + } + + fn write_config(home: &std::path::Path, content: &str) { + let dir = home.join(".config").join("opencode"); + std::fs::create_dir_all(&dir).expect("create config dir"); + std::fs::write(dir.join("opencode.json"), content).expect("write config"); + } + + #[test] + #[serial_test::serial] + fn read_rejects_non_object_root_instead_of_panicking_downstream() { + let temp = tempfile::tempdir().expect("tempdir"); + let _guard = TestHomeGuard::set(temp.path()); + + // 顶层数组/标量会让下游 `config["provider"] = …` 触发 serde_json panic。 + // 顶层 null 例外——serde_json 会把它自动升级成对象，本来就不炸。 + for malformed in ["[]", "[{\"a\":1}]", "42", "\"oops\""] { + write_config(temp.path(), malformed); + let result = read_opencode_config(); + assert!( + result.is_err(), + "non-object root must be rejected: {malformed}" + ); + } + + write_config(temp.path(), "{\"model\": \"x\"}"); + assert!( + read_opencode_config().is_ok(), + "a normal object config must still load" + ); + } + + #[test] + #[serial_test::serial] + fn set_mcp_server_normalizes_non_object_section() { + let temp = tempfile::tempdir().expect("tempdir"); + let _guard = TestHomeGuard::set(temp.path()); + + // `"mcp": []` 时旧代码的 as_object_mut 返回 None → 写入静默失效 + write_config(temp.path(), "{\"model\": \"keep-me\", \"mcp\": []}"); + + set_mcp_server("echo", json!({"command": "npx"})).expect("set must succeed"); + + let config = read_opencode_config().expect("reload"); + assert_eq!( + config["mcp"]["echo"]["command"], "npx", + "server must actually be written" + ); + assert_eq!( + config["model"], "keep-me", + "unrelated user config must be preserved" + ); + } +} diff --git a/src-tauri/src/provider.rs b/src-tauri/src/provider.rs index 04e6ab5c3..6569304b8 100644 --- a/src-tauri/src/provider.rs +++ b/src-tauri/src/provider.rs @@ -169,11 +169,23 @@ impl Provider { let api_key = first_non_empty(env, &["GEMINI_API_KEY", "GOOGLE_API_KEY"]); (base_url, api_key) } - AppType::GrokBuild => settings - .get("config") - .and_then(Value::as_str) - .and_then(crate::grok_config::extract_credentials) - .unwrap_or_default(), + // GrokBuild 的 base_url 与 api_key 必须各自解析：extract_credentials 在 + // 凭据缺失时整个 Option 变 None，一并 unwrap_or_default 会把明明写在 + // 配置里的 base_url 也清成空串。凭据缺失是常态（env_key 指向的变量在 + // GUI 进程里读不到），端点不该被连坐——否则用量脚本的 {{baseUrl}} 变成 + // 相对路径、余额查询只报「API key is empty」掩盖真因。 + // 与上面 Codex 分支的写法保持一致。 + AppType::GrokBuild => { + let config_text = settings.get("config").and_then(Value::as_str); + let base_url = config_text + .and_then(crate::grok_config::extract_base_url) + .unwrap_or_default(); + let api_key = config_text + .and_then(crate::grok_config::extract_credentials) + .map(|(_, api_key)| api_key) + .unwrap_or_default(); + (base_url, api_key) + } // Hermes (config.yaml) flattens credentials at the top level, snake_case. AppType::Hermes => ( str_at(settings.get("base_url")), diff --git a/src-tauri/src/proxy/providers/streaming_codex_anthropic.rs b/src-tauri/src/proxy/providers/streaming_codex_anthropic.rs index 1a4cd1d59..4f2b4d4e2 100644 --- a/src-tauri/src/proxy/providers/streaming_codex_anthropic.rs +++ b/src-tauri/src/proxy/providers/streaming_codex_anthropic.rs @@ -589,6 +589,21 @@ pub(crate) fn responses_sse_events_from_anthropic_message( tool_context: CodexToolContext, ) -> Vec { let mut state = AnthropicToResponsesState::with_tool_context(tool_context); + + // The whole conversion below assumes `body` is an Anthropic message object. + // A misbehaving gateway can return a top-level JSON array (or scalar) with + // HTTP 200 despite `stream:true`; index-assigning `message_start["content"]` + // on such a non-object would panic. Bail out gracefully instead. + if !body.is_object() { + return state + .failed_event( + "upstream returned a non-object Anthropic message body".to_string(), + Some("invalid_response".to_string()), + ) + .into_iter() + .collect(); + } + if body.get("type").and_then(Value::as_str) == Some("error") || body.get("error").is_some() { let (message, error_type) = extract_anthropic_sse_error(body); return state @@ -819,6 +834,15 @@ mod tests { assert!(!merged.contains("stream_truncated")); } + #[test] + fn test_json_non_object_body_returns_failed_event_not_panic() { + // A gateway that ignores `stream:true` and returns a top-level JSON array + // would have panicked on `message_start["content"] = …` before the guard. + let merged = render_message_events(&json!([1, 2, 3])); + assert!(merged.contains("event: response.failed")); + assert!(merged.contains("invalid_response")); + } + #[test] fn test_json_message_becomes_complete_responses_stream() { let merged = render_message_events(&json!({ diff --git a/src-tauri/src/proxy/providers/transform_codex_anthropic.rs b/src-tauri/src/proxy/providers/transform_codex_anthropic.rs index 74959da8d..18d247fef 100644 --- a/src-tauri/src/proxy/providers/transform_codex_anthropic.rs +++ b/src-tauri/src/proxy/providers/transform_codex_anthropic.rs @@ -1418,13 +1418,39 @@ pub fn anthropic_sse_to_message_value(body: &str) -> Result { }; match value.get("type").and_then(|t| t.as_str()).unwrap_or("") { "message_start" => { - if let Some(msg) = value.get("message") { + // Only accept an object message; a malformed upstream could send a + // scalar/array here, and the later `message["content"] = …` index + // assignment would panic on a non-object Value. + if let Some(msg) = value.get("message").filter(|m| m.is_object()) { *message = Some(msg.clone()); } } "content_block_start" => { if let Some(index) = value.get("index").and_then(|v| v.as_u64()) { - let block = value.get("content_block").cloned().unwrap_or(json!({})); + // Sanitize to an object: any later index-assignment (`["text"]`, + // `["signature"]`, `["input"]`) requires a JSON object, so a + // malformed non-object block from the upstream cannot be stored + // verbatim (it would panic on the next delta). + // + // The replacement carries `type: "text"` rather than being empty: + // the deltas that follow are usually well-formed, and a block with + // no `type` is silently dropped by the final Responses conversion, + // which turns a garbled block header into a `completed` response + // with empty output — the client sees the model saying nothing and + // has no way to tell that data was discarded. A text block recovers + // the common case; a tool-use block still yields nothing, exactly as + // it did before. + let block = match value.get("content_block") { + Some(block) if block.is_object() => block.clone(), + malformed => { + if malformed.is_some() { + log::warn!( + "Anthropic upstream sent a non-object content_block at index {index}; recovering it as a text block" + ); + } + json!({ "type": "text" }) + } + }; blocks.insert(index, block); json_accum.entry(index).or_default(); } @@ -2953,4 +2979,42 @@ data: {\"type\":\"content_block_start\",\"index\":0,\"content_block\":{\"type\": "data: {\"type\":\"message_start\",\"message\":{\"id\":\"m\",\"content\":[]}}\n\n"; assert!(anthropic_sse_to_message_value(sse).is_err()); } + + #[test] + fn test_anthropic_sse_aggregation_non_object_content_block_does_not_panic() { + // A malformed upstream can send a non-object `content_block`; the index + // assignment on the next delta would have panicked before the shape guard. + let sse = concat!( + "data: {\"type\":\"message_start\",\"message\":{\"id\":\"m\",\"content\":[]}}\n\n", + "data: {\"type\":\"content_block_start\",\"index\":0,\"content_block\":[1]}\n\n", + "data: {\"type\":\"content_block_delta\",\"index\":0,\"delta\":{\"type\":\"text_delta\",\"text\":\"x\"}}\n\n", + "data: {\"type\":\"message_stop\"}\n\n", + ); + let msg = anthropic_sse_to_message_value(sse) + .expect("aggregation must not panic on a non-object content_block"); + assert_eq!(msg["content"][0]["text"], json!("x")); + + // Not panicking is only half of it: the sanitized block must still carry a + // `type`, because the final conversion matches on it and silently drops + // anything it does not recognise. Asserting only on the intermediate value + // would pass while the client receives a `completed` response with empty + // output and no indication that the text was thrown away. + let response = anthropic_response_to_responses(msg).expect("final conversion must succeed"); + assert_eq!( + response["output"][0]["content"][0]["text"], + json!("x"), + "text recovered from a malformed block must survive to the Responses output: {response}" + ); + } + + #[test] + fn test_anthropic_sse_aggregation_non_object_message_errors_not_panic() { + // A malformed upstream can send a scalar `message`; the later + // `message["content"] = …` would have panicked before the shape guard. + let sse = concat!( + "data: {\"type\":\"message_start\",\"message\":\"oops\"}\n\n", + "data: {\"type\":\"message_stop\"}\n\n", + ); + assert!(anthropic_sse_to_message_value(sse).is_err()); + } } diff --git a/src-tauri/src/services/provider/mod.rs b/src-tauri/src/services/provider/mod.rs index 2d8e25468..7be078dc0 100644 --- a/src-tauri/src/services/provider/mod.rs +++ b/src-tauri/src/services/provider/mod.rs @@ -702,6 +702,469 @@ mod tests { ); } + #[test] + fn extract_gemini_common_config_strips_credentials_keeps_shareable() { + // Gemini 的共享片段会被 deep-merge 回**其它** Gemini 供应商的 env + // (live.rs::apply_common_config_to_settings)，因此任何凭据都不得进入片段。 + // 之前这里只硬编码跳过 GEMINI_API_KEY/GOOGLE_GEMINI_BASE_URL，而 + // GOOGLE_API_KEY 是 provider.rs 认可的一等 Gemini 凭据 → 会泄露到别的供应商。 + let settings = json!({ + "env": { + "GEMINI_API_KEY": "g-gem", + "GOOGLE_API_KEY": "g-legacy-real-key", + "GOOGLE_GEMINI_BASE_URL": "https://gemini.example", + "GOOGLE_APPLICATION_CREDENTIALS": "/path/creds.json", + "SOME_PROXY_AUTH_TOKEN": "tok-proxy", + // 可共享的非机密配置必须保留 + "GEMINI_TIMEOUT_MS": "30000" + } + }); + + let snippet = + ProviderService::extract_gemini_common_config(&settings).expect("extract should work"); + let value: Value = serde_json::from_str(&snippet).expect("snippet is valid JSON"); + + for leaked in [ + "GEMINI_API_KEY", + "GOOGLE_API_KEY", + "GOOGLE_APPLICATION_CREDENTIALS", + "SOME_PROXY_AUTH_TOKEN", + ] { + assert!( + value.get(leaked).is_none(), + "credential {leaked} must not leak into the shared Gemini snippet" + ); + } + assert_eq!( + value.get("GEMINI_TIMEOUT_MS").and_then(|v| v.as_str()), + Some("30000"), + "shareable non-secret config must be preserved" + ); + } + + /// 造一个「已被污染」的现场：片段里带 A 账号的凭据 + 一个合法可共享键。 + #[test] + fn sensitive_key_matcher_covers_common_credential_namings() { + for key in [ + // 裸 `_KEY`：最常见的写法，却曾被"只枚举 `_API_KEY` 这些子类"漏在外面 + "OPENAI_KEY", + "GROQ_KEY", + "XAI_KEY", + // 不带分隔符的复合写法 + "VOLC_ACCESSKEY", + "ALIYUN_SECRETKEY", + "SOME_APITOKEN", + // personal access token：既不含 TOKEN 也不含 KEY + "GITHUB_PAT", + "gitlab_pat", + // 口令类缩写 + "MYSQL_PWD", + "DB_PASS", + "GPG_PASSPHRASE", + "AWS_CREDS", + ] { + assert!( + ProviderService::is_sensitive_config_key(key), + "{key} must be treated as a credential" + ); + } + + // 后缀必须带下划线，不能把正常配置一起卷进来 + for key in [ + "PATH", + "OLDPWD", + "GEMINI_COMPAT", + "SSL_BYPASS", + "GEMINI_TIMEOUT_MS", + "CLAUDE_CODE_MAX_OUTPUT_TOKENS", + ] { + assert!( + !ProviderService::is_sensitive_config_key(key), + "{key} is ordinary shareable config and must not be stripped" + ); + } + } + + fn seed_leaked_gemini_state(db: &Arc) { + db.set_config_snippet( + "gemini", + Some( + json!({ + "GOOGLE_API_KEY": "key-A-leaked", + "SOME_PROXY_AUTH_TOKEN": "tok-A-leaked", + "GEMINI_TIMEOUT_MS": "30000" + }) + .to_string(), + ), + ) + .expect("seed snippet"); + + // 受害者 B：泄漏的密钥已经被合并进它的 env + let victim = Provider::with_id( + "b".into(), + "Relay B".into(), + json!({ "env": { + "GOOGLE_GEMINI_BASE_URL": "https://relay-b.example", + "GOOGLE_API_KEY": "key-A-leaked", + "GEMINI_TIMEOUT_MS": "30000" + }}), + None, + ); + db.save_provider("gemini", &victim).expect("save victim"); + + // 供应商 C：自己写了同名键但值不同，不能被误删 + let unrelated = Provider::with_id( + "c".into(), + "Own Key C".into(), + json!({ "env": { + "GOOGLE_GEMINI_BASE_URL": "https://c.example", + "GOOGLE_API_KEY": "key-C-owned" + }}), + None, + ); + db.save_provider("gemini", &unrelated).expect("save c"); + } + + #[tokio::test] + #[serial] + async fn scrub_gemini_removes_leaked_credentials_from_snippet_and_providers() { + let _home = TempHome::new(); + crate::settings::reload_settings().expect("reload settings"); + let db = Arc::new(Database::memory().expect("init db")); + let state = AppState::new(db.clone()); + seed_leaked_gemini_state(&db); + + ProviderService::scrub_leaked_gemini_common_config(&state) + .await + .expect("scrub must succeed"); + + // 片段：凭据清掉，可共享配置保留 + let snippet = db + .get_config_snippet("gemini") + .expect("read snippet") + .expect("snippet must still exist"); + let snippet: Value = serde_json::from_str(&snippet).expect("valid json"); + assert!(snippet.get("GOOGLE_API_KEY").is_none()); + assert!(snippet.get("SOME_PROXY_AUTH_TOKEN").is_none()); + assert_eq!( + snippet.get("GEMINI_TIMEOUT_MS").and_then(Value::as_str), + Some("30000"), + "shareable config must survive the scrub" + ); + + // 受害者 B：扩散过去的那一份被清掉 + let providers = db.get_all_providers("gemini").expect("providers"); + let victim_env = &providers["b"].settings_config["env"]; + assert!( + victim_env.get("GOOGLE_API_KEY").is_none(), + "leaked key must be removed from the victim provider" + ); + assert_eq!( + victim_env.get("GEMINI_TIMEOUT_MS").and_then(Value::as_str), + Some("30000"), + "non-credential config must not be touched" + ); + } + + #[tokio::test] + #[serial] + async fn scrub_gemini_keeps_a_providers_own_differently_valued_key() { + let _home = TempHome::new(); + crate::settings::reload_settings().expect("reload settings"); + let db = Arc::new(Database::memory().expect("init db")); + let state = AppState::new(db.clone()); + seed_leaked_gemini_state(&db); + + ProviderService::scrub_leaked_gemini_common_config(&state) + .await + .expect("scrub must succeed"); + + // 这条最容易写错成「按键名一刀切」：C 自己的密钥值与片段不同，是它自己的凭据 + let providers = db.get_all_providers("gemini").expect("providers"); + assert_eq!( + providers["c"].settings_config["env"] + .get("GOOGLE_API_KEY") + .and_then(Value::as_str), + Some("key-C-owned"), + "a provider's own key must not be deleted by name matching" + ); + } + + #[tokio::test] + #[serial] + async fn scrub_gemini_audit_records_key_names_but_never_values() { + let _home = TempHome::new(); + crate::settings::reload_settings().expect("reload settings"); + let db = Arc::new(Database::memory().expect("init db")); + let state = AppState::new(db.clone()); + seed_leaked_gemini_state(&db); + + ProviderService::scrub_leaked_gemini_common_config(&state) + .await + .expect("scrub must succeed"); + + let audit_text = db + .get_setting("gemini_common_config_scrub_audit_v1") + .expect("read audit") + .expect("an audit record must exist so the deletion is not silent"); + + // 值绝不能进这条记录：`settings` 会随 WebDAV/S3 同步上传，留值等于把一次 + // 清除换成一份跨设备扩散、没有界面入口、永不过期的明文副本。 + assert!( + !audit_text.contains("key-A-leaked") && !audit_text.contains("tok-A-leaked"), + "the audit record must never carry credential values: {audit_text}" + ); + + // 但必须说清楚删了什么、从哪删的，否则用户只能靠翻日志 + let audit: Value = serde_json::from_str(&audit_text).expect("audit is JSON"); + let removed: Vec<&str> = audit["removedFromSnippet"] + .as_array() + .expect("removedFromSnippet array") + .iter() + .filter_map(Value::as_str) + .collect(); + assert!( + removed.contains(&"GOOGLE_API_KEY") && removed.contains(&"SOME_PROXY_AUTH_TOKEN"), + "every key removed from the snippet must be named: {audit}" + ); + let victim = audit["providers"] + .as_array() + .expect("providers array") + .iter() + .find(|entry| entry["id"] == json!("b")) + .expect("every provider whose config gets rewritten must be recorded"); + assert_eq!( + victim["removedKeys"], + json!(["GOOGLE_API_KEY"]), + "the record must name what was taken from each provider: {audit}" + ); + } + + #[tokio::test] + #[serial] + async fn scrub_gemini_never_overwrites_an_existing_audit_record() { + let _home = TempHome::new(); + crate::settings::reload_settings().expect("reload settings"); + let db = Arc::new(Database::memory().expect("init db")); + let state = AppState::new(db.clone()); + seed_leaked_gemini_state(&db); + + // 上一轮改到一半就中止的情形：完成标记没置位，下次启动会重跑，但那时 + // 读到的"原始状态"已经残缺。无条件覆盖会拿残缺记录盖掉第一轮那份完整的。 + db.set_setting( + "gemini_common_config_scrub_audit_v1", + "{\"from\":\"an earlier, complete run\"}", + ) + .expect("seed an existing audit record"); + + ProviderService::scrub_leaked_gemini_common_config(&state) + .await + .expect("scrub must succeed"); + + assert_eq!( + db.get_setting("gemini_common_config_scrub_audit_v1") + .expect("read audit") + .as_deref(), + Some("{\"from\":\"an earlier, complete run\"}"), + "an audit record from an earlier run must survive a retry" + ); + } + + #[tokio::test] + #[serial] + async fn scrub_gemini_cleans_the_live_env_without_a_current_provider() { + let _home = TempHome::new(); + crate::settings::reload_settings().expect("reload settings"); + let db = Arc::new(Database::memory().expect("init db")); + let state = AppState::new(db.clone()); + seed_leaked_gemini_state(&db); + + // 没有当前供应商——这正是 sync_current_provider_for_app 直接返回 Ok 而 + // 根本不写文件的分支。此时 live 若清不掉，片段又已被清空，下次切换的 + // backfill 就会把残留永久写进受害供应商的配置。 + crate::gemini_config::write_gemini_env_atomic(&HashMap::from([ + ("GOOGLE_API_KEY".to_string(), "key-A-leaked".to_string()), + ("GEMINI_TIMEOUT_MS".to_string(), "30000".to_string()), + // 只存在于 live 的手工修改：定向删除必须保住它，全量重投影会抹掉 + ( + "HTTPS_PROXY".to_string(), + "http://127.0.0.1:7890".to_string(), + ), + ])) + .expect("seed live env"); + + ProviderService::scrub_leaked_gemini_common_config(&state) + .await + .expect("scrub must succeed"); + + let live = crate::gemini_config::read_gemini_env().expect("read live env"); + assert!( + !live.contains_key("GOOGLE_API_KEY"), + "the leaked credential must be gone from ~/.gemini/.env: {live:?}" + ); + assert_eq!( + live.get("HTTPS_PROXY").map(String::as_str), + Some("http://127.0.0.1:7890"), + "a hand-added live-only var must survive targeted removal: {live:?}" + ); + } + + #[tokio::test] + #[serial] + async fn scrub_gemini_live_cleanup_preserves_the_rest_of_the_env_file() { + let _home = TempHome::new(); + crate::settings::reload_settings().expect("reload settings"); + let db = Arc::new(Database::memory().expect("init db")); + let state = AppState::new(db.clone()); + seed_leaked_gemini_state(&db); + + // 这是一次用户没主动触发的启动期清理，不该顺手重写与泄漏无关的内容。 + // read→HashMap→write 的往返会把注释、空行、无法识别的行全丢掉并按键名重排。 + let original = "\ +# my own notes +GOOGLE_API_KEY=key-C-owned + +GOOGLE_API_KEY=key-A-leaked +this line is not KEY=VALUE at all +GEMINI_TIMEOUT_MS=30000 +"; + crate::gemini_config::write_gemini_env_text_atomic(original).expect("seed live env"); + + ProviderService::scrub_leaked_gemini_common_config(&state) + .await + .expect("scrub must succeed"); + + let raw = std::fs::read_to_string(crate::gemini_config::get_gemini_env_path()) + .expect("read live env"); + assert!( + !raw.contains("key-A-leaked"), + "the leaked line must be gone: {raw:?}" + ); + assert!( + raw.contains("# my own notes"), + "comments must survive a targeted removal: {raw:?}" + ); + assert!( + raw.contains("this line is not KEY=VALUE at all"), + "unparseable lines must survive a targeted removal: {raw:?}" + ); + // 被泄漏值遮住的那条重新生效——正是想要的结果，遮住它的恰恰是泄漏值 + assert_eq!( + crate::gemini_config::read_gemini_env() + .expect("read live env") + .get("GOOGLE_API_KEY") + .map(String::as_str), + Some("key-C-owned"), + "only the matching line may be dropped: {raw:?}" + ); + } + + #[tokio::test] + #[serial] + async fn scrub_gemini_aborts_before_clearing_the_snippet_when_the_live_backup_fails() { + let _home = TempHome::new(); + crate::settings::reload_settings().expect("reload settings"); + let db = Arc::new(Database::memory().expect("init db")); + let state = AppState::new(db.clone()); + seed_leaked_gemini_state(&db); + + // 关代理时这份快照会被原样写回 live。若清不动它却照样清了片段、置了完成标记， + // 代理一停凭据就复活，而一次性标记保证不会再清第二次。 + db.save_live_backup("gemini", "}not json{") + .await + .expect("seed backup"); + + let result = ProviderService::scrub_leaked_gemini_common_config(&state).await; + assert!( + result.is_err(), + "a backup that cannot be cleaned must abort the scrub" + ); + + // 片段是「该剥哪些键」的唯一知识来源，中止后必须原样留着，否则下次重试 + // 会因为 poison 为空而直接短路，反倒把标记置上 + let snippet = db + .get_config_snippet("gemini") + .expect("read snippet") + .expect("snippet must still exist"); + assert!( + snippet.contains("key-A-leaked"), + "the snippet must be left intact so the next boot can retry: {snippet}" + ); + assert!( + db.get_setting("gemini_common_config_credentials_scrubbed_v1") + .expect("read flag") + .is_none(), + "the one-shot flag must not be set when the scrub aborted" + ); + } + + #[tokio::test] + #[serial] + async fn scrub_gemini_leaves_no_residue_for_backfill_to_persist() { + let _home = TempHome::new(); + crate::settings::reload_settings().expect("reload settings"); + let db = Arc::new(Database::memory().expect("init db")); + let state = AppState::new(db.clone()); + seed_leaked_gemini_state(&db); + + ProviderService::scrub_leaked_gemini_common_config(&state) + .await + .expect("scrub must succeed"); + + // 顺序陷阱回归：如果只清了片段，切走供应商时 remove_common_config_from_settings + // 就不再认识这个键，live 里的残留会被 backfill 永久写进供应商配置。 + // 清理必须是原子的——清完之后，任何地方都不该再有那个值。 + let snippet = db + .get_config_snippet("gemini") + .expect("read snippet") + .unwrap_or_default(); + assert!(!snippet.contains("key-A-leaked")); + + for (id, provider) in db.get_all_providers("gemini").expect("providers") { + assert!( + !provider + .settings_config + .to_string() + .contains("key-A-leaked"), + "provider '{id}' still carries the leaked value" + ); + } + } + + #[tokio::test] + #[serial] + async fn scrub_gemini_is_idempotent_and_skips_on_second_run() { + let _home = TempHome::new(); + crate::settings::reload_settings().expect("reload settings"); + let db = Arc::new(Database::memory().expect("init db")); + let state = AppState::new(db.clone()); + seed_leaked_gemini_state(&db); + + ProviderService::scrub_leaked_gemini_common_config(&state) + .await + .expect("first run"); + + // 第二次必须是 no-op：用户清理后重新填的凭据不能被再抹一遍 + db.set_config_snippet( + "gemini", + Some(json!({"GOOGLE_API_KEY": "restored"}).to_string()), + ) + .expect("user re-adds a value"); + + ProviderService::scrub_leaked_gemini_common_config(&state) + .await + .expect("second run"); + + let snippet = db + .get_config_snippet("gemini") + .expect("read snippet") + .expect("snippet exists"); + assert!( + snippet.contains("restored"), + "the one-shot flag must prevent a second scrub: {snippet}" + ); + } + #[test] fn extract_claude_common_config_strips_all_credentials_keeps_shareable() { // env 混入多种凭据（Anthropic/OpenRouter/Google/OpenAI/Gemini + AWS/Vertex） @@ -3029,20 +3492,39 @@ impl ProviderService { /// `OPENROUTER_API_KEY` / `GOOGLE_API_KEY` 等回退）、各类 `*_AUTH_TOKEN` / /// 单数 `*_TOKEN`、AWS Bedrock / Vertex 凭据、以及通用 secret / password / /// 私钥命名。 - fn is_sensitive_config_key(name: &str) -> bool { + pub(crate) fn is_sensitive_config_key(name: &str) -> bool { let upper = name.to_ascii_uppercase(); // 单数 `_TOKEN` 命中 AWS_SESSION_TOKEN 等，但**不**误伤复数 `_TOKENS` // （CLAUDE_CODE_MAX_OUTPUT_TOKENS / MAX_THINKING_TOKENS 是正常可共享配置）。 const SENSITIVE_SUFFIXES: &[&str] = &[ + // 裸 `_KEY` 是最常见的凭据写法（OPENAI_KEY / GROQ_KEY / XAI_KEY…）， + // 必须单列：只枚举 `_API_KEY` / `_ACCESS_KEY` 这些子类，等于把最普通 + // 的那一种漏在外面。下面几条 `_*_KEY` 被它蕴含，保留是为了说明覆盖面。 + "_KEY", "_API_KEY", - "_APIKEY", - "_AUTH_TOKEN", - "_TOKEN", "_ACCESS_KEY", "_ACCESS_KEY_ID", "_KEY_ID", "_PRIVATE_KEY", + // 不带分隔符的复合写法各走各的后缀：`_KEY` 够不着 `..._APIKEY` + // （倒数第四个字符是 I 不是下划线）。VOLC_ACCESSKEY 是火山引擎文档 + // 里的正式变量名，本仓库就实现了火山 AK/SK 用量查询。 + "_APIKEY", + "_ACCESSKEY", + "_SECRETKEY", + "_APITOKEN", + "_AUTH_TOKEN", + "_TOKEN", + // GITHUB_PAT / GITLAB_PAT 等 personal access token 的惯用写法， + // 既不含 TOKEN 也不含 KEY，前面每一条规则都够不着。 + "_PAT", + // 口令类的常见缩写。`_PASS` 不会误伤 `*_BYPASS`（那个以 `_BYPASS` + // 结尾），`_PWD` 也不会误伤 shell 的 PWD / OLDPWD。 + "_PWD", + "_PASS", + "_PASSPHRASE", + "_CREDS", ]; const SENSITIVE_EXACT: &[&str] = &[ "APIKEY", @@ -3242,7 +3724,13 @@ impl ProviderService { let mut snippet = serde_json::Map::new(); if let Some(env) = env { for (key, value) in env { - if key == "GOOGLE_GEMINI_BASE_URL" || key == "GEMINI_API_KEY" { + // 端点按名剥离（它不是凭据，模式匹配够不着）；凭据全部交给 + // `is_sensitive_config_key` 统一模式匹配（与 Claude 提取器一致）。 + // 只列固定名单会漏掉下一个 `*_API_KEY` —— 例如 `GOOGLE_API_KEY` + // （provider.rs 认可的一等 Gemini 凭据），而共享片段会被 deep-merge + // 回其它 Gemini 供应商，漏剥即等于把 A 账号的密钥写进 B 供应商并 + // 发往 B 的 base_url。`GEMINI_API_KEY` 不必单列：`_KEY` 后缀已覆盖。 + if key == "GOOGLE_GEMINI_BASE_URL" || Self::is_sensitive_config_key(key) { continue; } let Value::String(v) = value else { @@ -3263,6 +3751,221 @@ impl ProviderService { .map_err(|e| AppError::Message(format!("Serialization failed: {e}"))) } + /// 一次性清理：把历史泄漏进 Gemini 共享片段的凭据从所有存储位置抹掉。 + /// + /// 背景：`extract_gemini_common_config` 曾只剥离两个固定键名，`GOOGLE_API_KEY` + /// 等一等凭据会进入共享片段，再被 `apply_common_config_to_settings` 深合并进 + /// **其它** Gemini 供应商的 env，随请求发往对方的 base_url。 + /// + /// 光修提取器不够：Gemini 的片段一旦生成就**永不自动重提取**（启动期 + /// auto-extract 与导入后补提取都要求 `snippet.is_none()`，切换时的回写又只对 + /// Claude / Codex 生效），所以存量片段会一直带着密钥继续注入。 + /// + /// 两个关键约束： + /// + /// 1. **不能只清片段**。合并与剥离是一对靠「值相等」严格抵消的操作：切走供应商时 + /// `remove_common_config_from_settings` 依据片段内容把注入的键删掉。片段里一旦 + /// 没了这个键，backfill 就会把 live 中残留的密钥原样写进受害供应商的 + /// `settings_config`——泄漏从瞬时污染变成永久污染。所以片段、各供应商配置、 + /// live 文件必须一起清。 + /// 2. **按值相等定向删除，不按键名一刀切**。复用 `remove_common_config_from_settings` + /// 可以只清掉扩散出去的那一份，保留某个供应商自己写的、值不同的同名键。 + /// + /// 步骤顺序本身是安全属性的一部分：**清片段必须排在最后**。片段是 + /// `remove_common_config_from_settings` 唯一的"该剥哪些键"来源，一旦清空，任何 + /// 残留（live 文件里的、下一轮重试要处理的）都再也无法被识别和剥离。所以所有 + /// 可能失败的步骤都排在它前面，失败即带错返回，让下次启动能原样重来。 + /// + /// 清理后部分供应商会显示缺少 API Key，需用户重填——这是正确行为：那把密钥本就 + /// 不属于它们。（受害者原有的同名键在合并时已被覆盖，无法恢复。）动手前会往 + /// settings 的 `gemini_common_config_scrub_audit_v1` 写一条审计记录，内容是 + /// **键名与受影响的供应商 id，不含值**：`settings` 会随 WebDAV/S3 同步上传， + /// 而这里处理的正是必须销毁的凭据，留值等于把一次清除换成一份跨设备扩散、 + /// 没有界面入口、永不过期的明文副本。 + pub async fn scrub_leaked_gemini_common_config(state: &AppState) -> Result<(), AppError> { + const FLAG: &str = "gemini_common_config_credentials_scrubbed_v1"; + const AUDIT_KEY: &str = "gemini_common_config_scrub_audit_v1"; + let app = AppType::Gemini; + + if state.db.get_bool_flag(FLAG).unwrap_or(false) { + return Ok(()); + } + + let Some(snippet_text) = state.db.get_config_snippet(app.as_str())? else { + state.db.set_setting(FLAG, "true")?; + return Ok(()); + }; + + // 片段解析不了就不动它，只标记完成——乱改用户数据比留着更糟 + let Ok(Value::Object(entries)) = serde_json::from_str::(&snippet_text) else { + state.db.set_setting(FLAG, "true")?; + return Ok(()); + }; + + let mut poison = serde_json::Map::new(); + let mut clean = serde_json::Map::new(); + for (key, value) in entries { + if Self::is_sensitive_config_key(&key) { + poison.insert(key, value); + } else { + clean.insert(key, value); + } + } + + if poison.is_empty() { + state.db.set_setting(FLAG, "true")?; + return Ok(()); + } + + log::warn!( + "检测到 {} 个凭据键残留在 Gemini 通用配置片段中，开始一次性清理", + poison.len() + ); + + let poison_keys: Vec = poison.keys().cloned().collect(); + let poison_value = Value::Object(poison); + let poison_text = serde_json::to_string(&poison_value) + .map_err(|e| AppError::Message(format!("Serialization failed: {e}")))?; + + // 1) 先算出各供应商清理后的配置，但**先不落库** + let providers = state.db.get_all_providers(app.as_str())?; + let mut pending: Vec<(String, Provider, Value)> = Vec::new(); + for (id, provider) in providers { + let cleaned = match live::remove_common_config_from_settings( + &app, + &provider.settings_config, + &poison_text, + ) { + Ok(cleaned) => cleaned, + Err(err) => { + log::warn!("清理供应商 '{id}' 的泄漏凭据失败: {err}"); + continue; + } + }; + if cleaned != provider.settings_config { + pending.push((id, provider, cleaned)); + } + } + + // 2) 落库前留一份审计记录：**只记键名与受影响的供应商，不记值**。 + // + // 「按值相等定向删除」在一种合法场景下也会命中：用户有意在多个供应商里 + // 复用同一把 key。所以必须留下"删了什么、从哪删的"，否则用户只能靠翻 + // 日志。但不能留值——`settings` 表不在 `SYNC_SKIP_TABLES` 里，会随 + // WebDAV/S3 同步上传，而这里处理的恰恰是必须销毁的泄漏凭据：留值等于 + // 把一次清除换成一份没有界面入口、永不过期、还会跨设备扩散的明文副本。 + // 密钥本来就该轮换，可恢复性不值这个代价。 + let removed_env_keys = |before: &Value, after: &Value| -> Vec { + let before_env = before.get("env").and_then(Value::as_object); + let after_env = after.get("env").and_then(Value::as_object); + match (before_env, after_env) { + (Some(before_env), Some(after_env)) => before_env + .keys() + .filter(|key| !after_env.contains_key(*key)) + .cloned() + .collect(), + (Some(before_env), None) => before_env.keys().cloned().collect(), + _ => Vec::new(), + } + }; + let audit = serde_json::json!({ + "removedFromSnippet": poison_keys, + "providers": pending + .iter() + .map(|(id, provider, cleaned)| serde_json::json!({ + "id": id, + "removedKeys": removed_env_keys(&provider.settings_config, cleaned), + })) + .collect::>(), + }); + let audit_text = serde_json::to_string(&audit) + .map_err(|e| AppError::Message(format!("Serialization failed: {e}")))?; + // 只在没有记录时写。provider 的写入不是一个事务（每次 save_provider 各自 + // 提交），上一轮可能改到一半就中止；此时完成标记没置位，下次启动会重跑， + // 而重跑看到的"原始状态"已经残缺。无条件 INSERT OR REPLACE 会拿这份残缺 + // 记录盖掉第一轮那份完整的。 + if state.db.get_setting(AUDIT_KEY)?.is_none() { + state.db.set_setting(AUDIT_KEY, &audit_text)?; + } + + // 3) 各供应商 settings_config：按值相等定向删除扩散出去的副本 + for (id, provider, cleaned) in pending { + let mut updated = provider; + updated.settings_config = cleaned; + state.db.save_provider(app.as_str(), &updated)?; + log::info!("已从 Gemini 供应商 '{id}' 中清除泄漏的共享凭据"); + } + + // 4) 代理接管中的 live 快照里也可能有一份副本。这一步的失败**必须传播**： + // + // 关代理时 `restore_live_config_for_app_with_fallback_inner`（proxy.rs:869） + // 会把这份快照原样写回 `~/.gemini/.env`。若它仍带毒而我们照样清了片段、置了 + // 完成标记，那么代理一停凭据就当场复活，而一次性标记又保证不会再清第二次； + // 此后片段里已没有这个键，下一次切换的 backfill 就把它永久写进受害供应商的 + // 配置——还是本函数开头那个顺序陷阱，只是换了扇门进来。 + // + // 带错返回是安全的失败方式：调用方（lib.rs:1189）只记 warn 不中断启动， + // 片段和标记都原样留着，下次启动照原样重来。 + if let Some(backup) = state.db.get_live_backup(app.as_str()).await? { + let original: Value = serde_json::from_str(&backup.original_config) + .map_err(|e| AppError::Message(format!("解析 Gemini 代理接管备份失败: {e}")))?; + let cleaned = live::remove_common_config_from_settings(&app, &original, &poison_text)?; + if cleaned != original { + let text = serde_json::to_string(&cleaned) + .map_err(|e| AppError::Message(format!("Serialization failed: {e}")))?; + state.db.save_live_backup(app.as_str(), &text).await?; + log::info!("已从 Gemini 代理接管备份中清除泄漏的共享凭据"); + } + } + + // 5) `~/.gemini/.env`：**定向**删除，且必须在清片段之前做，失败即中止。 + // + // 为什么不用 `sync_current_provider_for_app` 重投影：它在没有当前供应商 + // 时直接返回 Ok 而根本不写文件，泄漏值会原样留在 live 里；等片段被清空 + // 之后，下次切换时 `remove_common_config_from_settings` 再也认不出这个 + // 键，backfill 就把它永久写进受害供应商的配置——正是本函数开头说的那个 + // 顺序陷阱，只是由"没修"变成"修了一半更糟"。定向删除还顺带保住了只存在 + // 于 live、与供应商无关的手工 env（重投影会把它们抹掉）。 + // + // 删除走 `remove_gemini_env_entries` 的**保序**实现而不是 read→HashMap→ + // write 往返：后者会顺手抹掉注释、空行和无法识别的行，并按键名重排整个 + // 文件。全量投影时那无所谓，但这里是一次用户没主动触发的启动期清理，不该 + // 连带改写与泄漏无关的内容。 + // + // 失败就带着错误返回：片段此刻还留着毒键，完成标记也没置位，下次启动能 + // 照原样重来。清片段是不可逆的一步，必须排在所有会失败的步骤之后。 + let poison_env: HashMap = poison_value + .as_object() + .map(|map| { + map.iter() + .filter_map(|(key, value)| { + value.as_str().map(|text| (key.clone(), text.to_string())) + }) + .collect() + }) + .unwrap_or_default(); + if crate::gemini_config::remove_gemini_env_entries(&poison_env)? { + log::info!("已从 ~/.gemini/.env 中清除泄漏的共享凭据"); + } + + // 6) 片段本身：保留可共享的部分。全部清空时删行而不是写 "{}"——留着空行会让 + // should_auto_extract_config_snippet 永远为 false，用户的合法共享配置再也 + // 重建不回来。同理绝不置 cleared 标记。 + if clean.is_empty() { + state.db.set_config_snippet(app.as_str(), None)?; + } else { + let cleaned_snippet = serde_json::to_string_pretty(&Value::Object(clean)) + .map_err(|e| AppError::Message(format!("Serialization failed: {e}")))?; + state + .db + .set_config_snippet(app.as_str(), Some(cleaned_snippet))?; + } + + state.db.set_setting(FLAG, "true")?; + log::info!("Gemini 通用配置凭据清理完成"); + Ok(()) + } + /// Extract common config for OpenCode (JSON format) fn extract_opencode_common_config(settings: &Value) -> Result { // OpenCode uses a different config structure with npm, options, models diff --git a/src-tauri/src/services/skill.rs b/src-tauri/src/services/skill.rs index 85761c75f..e55506c6f 100644 --- a/src-tauri/src/services/skill.rs +++ b/src-tauri/src/services/skill.rs @@ -266,6 +266,26 @@ struct SkillBackupMetadata { const SKILL_BACKUP_RETAIN_COUNT: usize = 20; +/// 仓库归档解压上限：条目数与解压后总字节数。 +/// +/// 归档字节由第三方完全控制（仓库可经 deeplink 添加，且 branch 可把下载落点 +/// 改写到攻击者自传的 release asset），没有上限时一个几 MB 的压缩炸弹就能塞满磁盘。 +/// 取值对齐 `webdav_sync/archive.rs` 里同款保护的量级。 +const MAX_ARCHIVE_ENTRIES: usize = 10_000; +const MAX_ARCHIVE_TOTAL_BYTES: u64 = 512 * 1024 * 1024; +/// symlink 目标就是一条路径，几十字节就够；给到 4 KiB 是宽松上限。 +/// 必须有这个上限：zip 2.4.2 的 `make_reader` 不按声明的 uncompressed_size +/// 截断读取，所以一个打了 symlink 标志、deflate 流却能膨胀到数 GB 的条目， +/// 会被 `read_to_string` 整个读进内存。 +const MAX_SYMLINK_TARGET_BYTES: u64 = 4 * 1024; +/// 物化一个目录按一个目录块计费。空目录不写内容字节，但照样吃 inode 和磁盘块， +/// 不计费就等于允许无限量地造目录。 +const DIRECTORY_BUDGET_COST: u64 = 4096; +/// 压缩体上限。解压预算只有在 ZipArchive 建起来之后才生效，而那时整个响应体 +/// 已经在内存里了，所以下载这一步需要自己的上限。技能仓库是 Markdown， +/// 128 MiB 的压缩包已经远超正常规模。 +const MAX_ARCHIVE_DOWNLOAD_BYTES: u64 = 128 * 1024 * 1024; + /// 技能元数据 (从 SKILL.md 解析) #[derive(Debug, Clone, Deserialize)] pub struct SkillMetadata { @@ -447,8 +467,22 @@ impl SkillService { } /// 构建 Skill 文档 URL（指向仓库中的 SKILL.md 文件） - fn build_skill_doc_url(owner: &str, repo: &str, branch: &str, doc_path: &str) -> String { - format!("https://github.com/{owner}/{repo}/blob/{branch}/{doc_path}") + /// + /// 坐标不合法时返回 None：这个值会存进 `readme_url`，前端「查看文档」用 + /// `openExternal` 直接打开，恶意 branch 能把它指到 github.com 上的任意路径。 + fn build_skill_doc_url( + owner: &str, + repo: &str, + branch: &str, + doc_path: &str, + ) -> Option { + if Self::validate_repo_ref(owner, repo, branch).is_err() { + log::warn!("跳过非法仓库坐标的文档链接: {owner}/{repo}@{branch}"); + return None; + } + Some(format!( + "https://github.com/{owner}/{repo}/blob/{branch}/{doc_path}" + )) } /// 从旧 readme_url 中提取仓库内文档路径，兼容 `blob`/`tree` 两种格式 @@ -650,7 +684,7 @@ impl SkillService { }; // 下载仓库 - let (temp_dir, used_branch) = timeout( + let (temp_guard, used_branch) = timeout( std::time::Duration::from_secs(60), self.download_repo(&repo), ) @@ -666,13 +700,13 @@ impl SkillService { Some("checkNetwork"), )) })??; + let temp_dir = temp_guard.path(); repo_branch = used_branch; // 复制到 SSOT let source = - Self::resolve_skill_source_dir(&temp_dir, &skill.directory).ok_or_else(|| { + Self::resolve_skill_source_dir(temp_dir, &skill.directory).ok_or_else(|| { let missing = temp_dir.join(&source_rel).display().to_string(); - let _ = fs::remove_dir_all(&temp_dir); anyhow!(format_skill_error( "SKILL_DIR_NOT_FOUND", &[("path", &missing)], @@ -680,7 +714,9 @@ impl SkillService { )) })?; - let canonical_temp = temp_dir.canonicalize().unwrap_or_else(|_| temp_dir.clone()); + let canonical_temp = temp_dir + .canonicalize() + .unwrap_or_else(|_| temp_dir.to_path_buf()); let canonical_source = source.canonicalize().map_err(|_| { anyhow!(format_skill_error( "SKILL_DIR_NOT_FOUND", @@ -689,7 +725,6 @@ impl SkillService { )) })?; if !canonical_source.starts_with(&canonical_temp) || !canonical_source.is_dir() { - let _ = fs::remove_dir_all(&temp_dir); return Err(anyhow!(format_skill_error( "INVALID_SKILL_DIRECTORY", &[("directory", &skill.directory)], @@ -698,7 +733,6 @@ impl SkillService { } Self::copy_dir_recursive(&canonical_source, &dest)?; - let _ = fs::remove_dir_all(&temp_dir); // 使用实际下载成功的分支，避免 readme_url / repo_branch 与真实分支不一致。 if repo_branch != skill.repo_branch { @@ -725,12 +759,8 @@ impl SkillService { }) .unwrap_or_else(|| format!("{}/SKILL.md", skill.directory.trim_end_matches('/'))); - let readme_url = Some(Self::build_skill_doc_url( - &skill.repo_owner, - &skill.repo_name, - &repo_branch, - &doc_path, - )); + let readme_url = + Self::build_skill_doc_url(&skill.repo_owner, &skill.repo_name, &repo_branch, &doc_path); // 创建 InstalledSkill 记录 // 计算内容哈希 @@ -785,20 +815,40 @@ impl SkillService { .get_installed_skill(id)? .ok_or_else(|| anyhow!("Skill not found: {id}"))?; - let backup_path = - Self::create_uninstall_backup(&skill)?.map(|path| path.to_string_lossy().to_string()); + // DB 行可能被同步导入污染（远端快照 raw SQL 直接灌库，绕过安装期校验）， + // 也可能是 v3.11.0 引入 sanitize_install_name 之前留下的存量脏值 + // （当年扫描不过滤点开头目录，`.github/SKILL.md` 会存成 `.github`）。 + // + // 守卫失败时**跳过全部文件系统操作、但仍删除 DB 行**：`db.delete_skill` + // 全项目只有这一处调用且未暴露为命令，若在此直接返回 Err，用户就再也无法 + // 从界面删掉这条记录，只能手改 SQLite。安全目标是「不碰危险路径」， + // 不是「把用户锁在坏状态里」。 + let backup_path = match Self::require_valid_directory(&skill.directory) { + Ok(directory) => { + let backup_path = Self::create_uninstall_backup(&skill)? + .map(|path| path.to_string_lossy().to_string()); - // 从所有应用目录删除 - for app in AppType::all() { - let _ = Self::remove_from_app(&skill.directory, &app); - } + // 从所有应用目录删除 + for app in AppType::all() { + let _ = Self::remove_from_app(&directory, &app); + } - // 从 SSOT 删除 - let ssot_dir = Self::get_ssot_dir()?; - let skill_path = ssot_dir.join(&skill.directory); - if skill_path.exists() { - fs::remove_dir_all(&skill_path)?; - } + // 从 SSOT 删除 + let ssot_dir = Self::get_ssot_dir()?; + let skill_path = ssot_dir.join(&directory); + if skill_path.exists() { + fs::remove_dir_all(&skill_path)?; + } + backup_path + } + Err(err) => { + log::warn!( + "Skill {id} 的 directory 非法（{:?}），跳过文件清理，仅删除数据库记录: {err}", + skill.directory + ); + None + } + }; // 从数据库删除 db.delete_skill(id)?; @@ -900,7 +950,7 @@ impl SkillService { }; // 下载仓库 ZIP - let (temp_dir, _used_branch) = match timeout( + let (temp_guard, _used_branch) = match timeout( std::time::Duration::from_secs(60), self.download_repo(&repo), ) @@ -916,10 +966,11 @@ impl SkillService { continue; } }; + let temp_dir = temp_guard.path(); // 扫描仓库中的所有 Skill 目录 let mut remote_skills: Vec = Vec::new(); - let _ = self.scan_dir_recursive(&temp_dir, &temp_dir, &repo, &mut remote_skills); + let _ = self.scan_dir_recursive(temp_dir, temp_dir, &repo, &mut remote_skills); for skill in group_skills { // 在远程仓库中找到匹配的 Skill 目录 @@ -931,7 +982,7 @@ impl SkillService { }); let remote_skill_dir = match remote_match { - Some(rs) => match Self::resolve_skill_source_dir(&temp_dir, &rs.directory) { + Some(rs) => match Self::resolve_skill_source_dir(temp_dir, &rs.directory) { Some(path) => path, None => continue, }, @@ -949,20 +1000,28 @@ impl SkillService { // 本地哈希：优先数据库，否则实时计算 let local_hash = match &skill.content_hash { Some(h) => Some(h.clone()), - None => { - let local_dir = ssot_dir.join(&skill.directory); - if local_dir.exists() { - match Self::compute_dir_hash(&local_dir) { - Ok(h) => { - let _ = db.update_skill_hash(&skill.id, &h, 0); - Some(h) - } - Err(_) => None, - } - } else { + // 脏 directory 会让 compute_dir_hash 递归遍历任意目录， + // 且哈希结果经「有无更新」的界面状态泄露少量信息。 + None => match Self::require_valid_directory(&skill.directory) { + Err(err) => { + log::warn!("跳过非法 directory 的哈希计算: {err}"); None } - } + Ok(directory) => { + let local_dir = ssot_dir.join(&directory); + if local_dir.exists() { + match Self::compute_dir_hash(&local_dir) { + Ok(h) => { + let _ = db.update_skill_hash(&skill.id, &h, 0); + Some(h) + } + Err(_) => None, + } + } else { + None + } + } + }, }; if local_hash.as_deref() != Some(&remote_hash) { @@ -974,8 +1033,6 @@ impl SkillService { }); } } - - let _ = fs::remove_dir_all(&temp_dir); } Ok(updates) @@ -987,6 +1044,11 @@ impl SkillService { .get_installed_skill(skill_id)? .ok_or_else(|| anyhow!("Skill not found: {skill_id}"))?; + // 本函数后续三种危险操作都用 directory 拼路径：备份源（把任意目录复制进 + // 备份区并在界面列出）、remove_dir_all（删任意目录）、copy_dir_recursive + // （把远端仓库内容写到任意路径）。校验必须在这三者之前。 + Self::require_valid_directory(&skill.directory)?; + let (owner, name, branch) = match (&skill.repo_owner, &skill.repo_name) { (Some(o), Some(n)) => ( o.clone(), @@ -1009,7 +1071,7 @@ impl SkillService { let ssot_dir = Self::get_ssot_dir()?; // 下载仓库 - let (temp_dir, used_branch) = timeout( + let (temp_guard, used_branch) = timeout( std::time::Duration::from_secs(60), self.download_repo(&repo), ) @@ -1021,10 +1083,11 @@ impl SkillService { Some("checkNetwork"), )) })??; + let temp_dir = temp_guard.path(); // 在解压的仓库中查找 Skill 源目录 let mut remote_skills: Vec = Vec::new(); - let _ = self.scan_dir_recursive(&temp_dir, &temp_dir, &repo, &mut remote_skills); + let _ = self.scan_dir_recursive(temp_dir, temp_dir, &repo, &mut remote_skills); let remote_match = remote_skills .iter() @@ -1033,7 +1096,6 @@ impl SkillService { remote_install_name.eq_ignore_ascii_case(&skill.directory) }) .ok_or_else(|| { - let _ = fs::remove_dir_all(&temp_dir); anyhow!(format_skill_error( "SKILL_DIR_NOT_FOUND", &[("path", &skill.directory)], @@ -1041,10 +1103,9 @@ impl SkillService { )) })?; - let source = Self::resolve_skill_source_dir(&temp_dir, &remote_match.directory) - .ok_or_else(|| { + let source = + Self::resolve_skill_source_dir(temp_dir, &remote_match.directory).ok_or_else(|| { let missing = temp_dir.join(&remote_match.directory).display().to_string(); - let _ = fs::remove_dir_all(&temp_dir); anyhow!(format_skill_error( "SKILL_DIR_NOT_FOUND", &[("path", &missing)], @@ -1061,7 +1122,6 @@ impl SkillService { fs::remove_dir_all(&dest)?; } Self::copy_dir_recursive(&source, &dest)?; - let _ = fs::remove_dir_all(&temp_dir); // 计算新哈希 + 解析新元数据 let new_hash = Self::compute_dir_hash(&dest).ok(); @@ -1074,12 +1134,7 @@ impl SkillService { .as_deref() .and_then(Self::extract_doc_path_from_url) .unwrap_or_else(|| format!("{}/SKILL.md", skill.directory.trim_end_matches('/'))); - let readme_url = Some(Self::build_skill_doc_url( - &owner, - &name, - &used_branch, - &doc_path, - )); + let readme_url = Self::build_skill_doc_url(&owner, &name, &used_branch, &doc_path); let updated_skill = InstalledSkill { id: skill.id.clone(), @@ -1119,7 +1174,11 @@ impl SkillService { if skill.content_hash.is_some() { continue; } - let skill_dir = ssot_dir.join(&skill.directory); + let Ok(directory) = Self::require_valid_directory(&skill.directory) else { + log::warn!("跳过非法 directory 的哈希回填: {:?}", skill.directory); + continue; + }; + let skill_dir = ssot_dir.join(&directory); if !skill_dir.exists() { continue; } @@ -1175,8 +1234,20 @@ impl SkillService { }; for skill in skills.values() { - let src = old_dir.join(&skill.directory); - let dst = new_dir.join(&skill.directory); + // 下面是 rename 与 remove_dir_all，脏 directory 可把任意目录搬走或删掉。 + // 软失败：本函数已有 errors 收集通道，记一条继续处理其余 skill， + // 不要整体中断——用户只是在切换存储位置。 + let directory = match Self::require_valid_directory(&skill.directory) { + Ok(directory) => directory, + Err(err) => { + result + .errors + .push(format!("{}: {err}", skill.directory.escape_debug())); + continue; + } + }; + let src = old_dir.join(&directory); + let dst = new_dir.join(&directory); if !src.exists() { result.skipped_count += 1; @@ -1302,8 +1373,12 @@ impl SkillService { )); } + // meta.json 是文件内容（可能来自手工放置或不可信备份），directory 此前 + // 未经任何校验就直接 join——可穿越出 SSOT 目录写任意位置。必须先校验。 + let directory = Self::require_valid_directory(&metadata.skill.directory)?; + let ssot_dir = Self::get_ssot_dir()?; - let restore_path = ssot_dir.join(&metadata.skill.directory); + let restore_path = ssot_dir.join(&directory); if restore_path.exists() || Self::is_symlink(&restore_path) { return Err(anyhow!( "Restore target already exists: {}", @@ -1312,6 +1387,7 @@ impl SkillService { } let mut restored_skill = metadata.skill; + restored_skill.directory = directory; restored_skill.installed_at = Utc::now().timestamp(); restored_skill.apps = SkillApps::only(current_app); restored_skill.updated_at = 0; @@ -1465,7 +1541,16 @@ impl SkillService { search_sources.push((ssot_dir.clone(), "cc-switch".to_string())); for selection in imports { - let dir_name = selection.directory; + // selection.directory 由前端 IPC 直接传入、此前全程无校验，而它既被 + // 用来探测源目录、又作为 copy_dir_recursive 的目标、最后还原样入库。 + // 在入口处拒掉，同时切断「脏值 sink」和「脏值来源」两条线。 + let dir_name = match Self::require_valid_directory(&selection.directory) { + Ok(dir_name) => dir_name, + Err(err) => { + log::warn!("跳过导入：{err}"); + continue; + } + }; // 在所有候选目录中查找 let mut source_path: Option = None; @@ -1581,22 +1666,25 @@ impl SkillService { return Ok(()); } - let ssot_dir = Self::get_ssot_dir()?; - let source = ssot_dir.join(directory); + // directory 可能来自被污染的 DB 行（如同步导入的远端快照），join 前必须校验。 + let directory = Self::require_valid_directory(directory)?; - Self::validate_sync_source_dir(&source, directory)?; + let ssot_dir = Self::get_ssot_dir()?; + let source = ssot_dir.join(&directory); + + Self::validate_sync_source_dir(&source, &directory)?; let app_dir = Self::get_app_skills_dir(app)?; fs::create_dir_all(&app_dir)?; - let dest = app_dir.join(directory); + let dest = app_dir.join(&directory); let sync_method = Self::get_sync_method(); match sync_method { SyncMethod::Auto => { if dest.exists() && !Self::is_symlink(&dest) { - Self::replace_dest_with_copy(&source, &dest, directory)?; + Self::replace_dest_with_copy(&source, &dest, &directory)?; log::debug!("Skill {directory} 已通过复制同步到 {app:?}"); return Ok(()); } @@ -1620,7 +1708,7 @@ impl SkillService { } } // Fallback 到 copy - Self::replace_dest_with_copy(&source, &dest, directory)?; + Self::replace_dest_with_copy(&source, &dest, &directory)?; log::debug!("Skill {directory} 已通过复制同步到 {app:?}"); } SyncMethod::Symlink => { @@ -1631,7 +1719,7 @@ impl SkillService { log::debug!("Skill {directory} 已通过 symlink 同步到 {app:?}"); } SyncMethod::Copy => { - Self::replace_dest_with_copy(&source, &dest, directory)?; + Self::replace_dest_with_copy(&source, &dest, &directory)?; log::debug!("Skill {directory} 已通过复制同步到 {app:?}"); } } @@ -1753,8 +1841,12 @@ impl SkillService { return Ok(()); } + // directory 可能来自被污染的 DB 行（如同步导入的远端快照）， + // 这里执行的是删除操作，join 前必须校验，防止任意目录删除。 + let directory = Self::require_valid_directory(directory)?; + let app_dir = Self::get_app_skills_dir(app)?; - let skill_path = app_dir.join(directory); + let skill_path = app_dir.join(&directory); if skill_path.exists() || Self::is_symlink(&skill_path) { Self::remove_path(&skill_path)?; @@ -1804,7 +1896,15 @@ impl SkillService { for skill in skills.values() { if skill.apps.is_enabled_for(app) { - Self::sync_to_app_dir(&skill.directory, app)?; + // 逐条容错而非 `?` 传播：本函数在切换供应商时被调用，一条脏 + // directory（存量点开头目录、或同步导入灌进来的行）不得让整个 + // 应用的 skill 同步全部失效。 + if let Err(err) = Self::sync_to_app_dir(&skill.directory, app) { + log::warn!( + "同步 skill {} 到 {app:?} 失败，跳过该条: {err}", + skill.directory + ); + } } } @@ -1913,7 +2013,7 @@ impl SkillService { /// 从仓库获取技能列表 async fn fetch_repo_skills(&self, repo: &SkillRepo) -> Result> { - let (temp_dir, resolved_branch) = + let (temp_guard, resolved_branch) = timeout(std::time::Duration::from_secs(60), self.download_repo(repo)) .await .map_err(|_| { @@ -1929,12 +2029,10 @@ impl SkillService { })??; let mut skills = Vec::new(); - let scan_dir = temp_dir.clone(); + let scan_dir = temp_guard.path(); let mut resolved_repo = repo.clone(); resolved_repo.branch = resolved_branch; - self.scan_dir_recursive(&scan_dir, &scan_dir, &resolved_repo, &mut skills)?; - - let _ = fs::remove_dir_all(&temp_dir); + self.scan_dir_recursive(scan_dir, scan_dir, &resolved_repo, &mut skills)?; Ok(skills) } @@ -2002,12 +2100,7 @@ impl SkillService { name: meta.name.unwrap_or_else(|| directory.to_string()), description: meta.description.unwrap_or_default(), directory: directory.to_string(), - readme_url: Some(Self::build_skill_doc_url( - &repo.owner, - &repo.name, - &repo.branch, - doc_path, - )), + readme_url: Self::build_skill_doc_url(&repo.owner, &repo.name, &repo.branch, doc_path), repo_owner: repo.owner.clone(), repo_name: repo.name.clone(), repo_branch: repo.branch.clone(), @@ -2095,6 +2188,13 @@ impl SkillService { return None; } + // 显式拒绝两种分隔符，不能依赖 components() 的平台语义： + // `\` 在 Linux/macOS 上不是分隔符，会被当成合法单段名放行， + // 但同一个值同步/还原到 Windows 上就变成了嵌套路径。 + if trimmed.contains('/') || trimmed.contains('\\') { + return None; + } + let path = Path::new(trimmed); let mut components = path.components(); match (components.next(), components.next()) { @@ -2114,6 +2214,132 @@ impl SkillService { } } + /// 校验来自 DB 行 / 备份 meta.json 等外部来源的 directory 字段。 + /// + /// 存储值按构造本应是单段安装名（见 sanitize_install_name），但有两个入口 + /// 会绕过安装期校验：同步导入的远端快照直接灌库（raw SQL），以及手工放置 / + /// 不可信备份里的 meta.json。任何把它 join 进文件系统路径的使用点（尤其是 + /// remove_dir_all 这类删除操作）必须先过这道校验，拒绝路径穿越。 + /// + /// 只校验、不归一化：`sanitize_install_name` 会 `trim()`，若拿它的返回值替换 + /// 原值，磁盘上真实带空格的目录名就再也 join 不中。所以这里要求归一化结果与 + /// 原值逐字相同，否则一律视为非法。 + fn require_valid_directory(directory: &str) -> Result { + match Self::sanitize_install_name(directory) { + Some(normalized) if normalized == directory => Ok(normalized), + _ => Err(anyhow!( + "Invalid skill directory (possible path traversal): {directory:?}" + )), + } + } + + /// GitHub 账号名（user / org login）。 + /// + /// 只放行 ASCII 字母数字与 `-`。这比 GitHub 自身的规则更严，但该字段会被拼进 + /// 下载 URL，任何 `/`、`.`、`%`、`\` 都可能改写请求落点（见 validate_repo_ref）。 + fn is_valid_github_owner(owner: &str) -> bool { + !owner.is_empty() + && owner.len() <= 39 + && owner.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') + } + + /// GitHub 仓库名。允许 `.` `-` `_`，但整体不能是 `.` 或 `..`。 + fn is_valid_github_repo_name(name: &str) -> bool { + !name.is_empty() + && name.len() <= 100 + && name != "." + && name != ".." + && name + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '_')) + } + + /// git 分支名。 + /// + /// 分支名合法含 `/`（`feature/x`），所以不能整体禁掉分隔符——按段做白名单。 + /// 逐段 `!starts_with('.')` 比整体 `contains("..")` 更稳：它同时挡掉 `a/./b`、 + /// `a/.../b` 这类变形。除 `git check-ref-format` 的规则外还额外禁掉 `#` 与 `%`： + /// 前者会把 URL 后半截变成 fragment，后者可用百分号编码绕过字符检查。 + fn is_valid_git_branch(branch: &str) -> bool { + // 空串和 "HEAD" 都是 `download_repo` 的哨兵，语义都是「用仓库默认分支」： + // 分支候选表对两者一视同仁地跳过，改试 main / master，所以它们**永远不会 + // 被拼进 URL**，也就没有可校验的攻击面。空串必须放行——`skill_repos` 的 + // 存量行可以是空 branch（建表默认值是 'main'，但不禁止空串），前端两处 + // `repo.branch || "main"` 就是照着这个前提写的。把它当非法会让那些仓库 + // 在 download_repo 第一行就报 INVALID_REPO_REF，技能面板直接列不出来。 + if branch.is_empty() || branch.eq_ignore_ascii_case("HEAD") { + return true; + } + if branch.len() > 255 { + return false; + } + if branch.starts_with('/') || branch.ends_with('/') || branch.contains("//") { + return false; + } + if branch.contains("@{") { + return false; + } + // `is_ascii_control()` 的范围是 U+0000..=U+001F **加上** U+007F DELETE， + // 所以不需要另外再点名 DEL。 + if branch + .chars() + .any(|c| c.is_ascii_control() || " ~^:?*[\\#%".contains(c)) + { + return false; + } + branch.split('/').all(|segment| { + !segment.is_empty() + && !segment.starts_with('.') + && !segment.ends_with('.') + && !segment.ends_with(".lock") + }) + } + + /// 校验一组仓库坐标，用于任何会被拼进 github.com URL 的地方。 + /// + /// 动机：`download_repo` 把 owner/name/branch 直接 format 进 + /// `https://github.com/{owner}/{name}/archive/refs/heads/{branch}.zip`，而 URL + /// 解析会消解点段——branch 写成 `../../../releases/download/v1/evil` 时，落点变成 + /// 该仓库的 **release asset**，即攻击者可上传的任意字节。归档内容一旦可控， + /// 解压路径校验就成了唯一防线，所以这一层必须堵死。 + pub(crate) fn validate_repo_ref(owner: &str, name: &str, branch: &str) -> Result<()> { + if !Self::is_valid_github_owner(owner) || !Self::is_valid_github_repo_name(name) { + return Err(anyhow!(format_skill_error( + "INVALID_REPO_REF", + &[("owner", owner), ("name", name)], + Some("checkRepoUrl"), + ))); + } + if !Self::is_valid_git_branch(branch) { + return Err(anyhow!(format_skill_error( + "INVALID_REPO_REF", + &[("owner", owner), ("name", name), ("branch", branch)], + Some("checkRepoUrl"), + ))); + } + Ok(()) + } + + /// 出口断言：URL 拼好后再确认它确实指向预期的 github.com 路径。 + /// + /// 这是纵深防御——即便上面的字符集校验将来漏了某种变形（百分号编码、新的 + /// 分隔符语义等），这里也能拦住落点被改写的请求。 + fn assert_github_archive_url(url: &str, owner: &str, name: &str) -> Result<()> { + let parsed = url::Url::parse(url).map_err(|e| anyhow!("Invalid archive URL: {e}"))?; + let expected_prefix = format!("/{owner}/{name}/archive/refs/heads/"); + if parsed.scheme() != "https" + || parsed.host_str() != Some("github.com") + || !parsed.path().starts_with(&expected_prefix) + { + return Err(anyhow!(format_skill_error( + "INVALID_REPO_REF", + &[("owner", owner), ("name", name)], + Some("checkRepoUrl"), + ))); + } + Ok(()) + } + /// 在目录树中查找名称匹配且包含 SKILL.md 的子目录 /// /// 用于 skills.sh 安装回退：API 只返回 skillId（如 "find-skills"）， @@ -2197,10 +2423,18 @@ impl SkillService { } /// 下载仓库 - async fn download_repo(&self, repo: &SkillRepo) -> Result<(PathBuf, String)> { + /// + /// 这里是仓库坐标进入 URL 的**唯一收敛点**——`fetch_repo_skills`、`install`、 + /// `check_updates`、`update_skill` 四条路径都经过它，而 `skill_repos` / `skills` + /// 两张表都会被同步导入的远端快照整表覆盖，入库校验管不住它们。所以主防线放这里。 + async fn download_repo(&self, repo: &SkillRepo) -> Result<(tempfile::TempDir, String)> { + Self::validate_repo_ref(&repo.owner, &repo.name, &repo.branch)?; + + // 守卫全程持有，成功后连同目录一起交给调用方（见 `extract_local_zip` 的说明）。 + // 原来这里立刻 keep()，任何一步失败——下载超时、ARCHIVE_TOO_LARGE、解压出错 + // ——都会把半个解压目录永久留在磁盘上，反复触发即可持续填盘。 let temp_dir = tempfile::tempdir()?; let temp_path = temp_dir.path().to_path_buf(); - let _ = temp_dir.keep(); let mut branches = Vec::new(); if !repo.branch.is_empty() && !repo.branch.eq_ignore_ascii_case("HEAD") { @@ -2219,12 +2453,15 @@ impl SkillService { "https://github.com/{}/{}/archive/refs/heads/{}.zip", repo.owner, repo.name, branch ); + Self::assert_github_archive_url(&url, &repo.owner, &repo.name)?; match self.download_and_extract(&url, &temp_path).await { - Ok(_) => { - return Ok((temp_path, branch.to_string())); - } + Ok(_) => return Ok((temp_dir, branch.to_string())), Err(e) => { + // 每个分支各自重算预算，所以失败后必须把上一轮的残留清掉—— + // 否则 N 个候选分支等于 N 倍的落盘量堆在同一个目录里。 + let _ = fs::remove_dir_all(&temp_path); + let _ = fs::create_dir_all(&temp_path); last_error = Some(e); continue; } @@ -2252,10 +2489,114 @@ impl SkillService { ))); } - let bytes = response.bytes().await?; - let cursor = std::io::Cursor::new(bytes); - let mut archive = zip::ZipArchive::new(cursor)?; + // 逐块读并卡住压缩体大小：`response.bytes()` 会先把攻击者控制的整个归档 + // 收进内存，之后才轮到 ZipArchive 和解压预算——那时候堆已经被吃光了。 + // 不能只信 Content-Length（可以撒谎或缺失），必须按实际收到的字节数算。 + let mut response = response; + let mut body: Vec = Vec::new(); + while let Some(chunk) = response.chunk().await? { + if body.len().saturating_add(chunk.len()) as u64 > MAX_ARCHIVE_DOWNLOAD_BYTES { + let limit_mb = (MAX_ARCHIVE_DOWNLOAD_BYTES / 1024 / 1024).to_string(); + return Err(anyhow::anyhow!(format_skill_error( + "ARCHIVE_TOO_LARGE", + &[("limit_mb", &limit_mb)], + Some("checkZipContent"), + ))); + } + body.extend_from_slice(&chunk); + } + let cursor = std::io::Cursor::new(body); + let archive = zip::ZipArchive::new(cursor)?; + Self::extract_repo_archive(archive, dest) + } + + /// 按预算把单个归档条目写出，累计超限即中止。 + /// + /// 逐块累加而非读取归档头里声明的 size：那个值由归档作者填写，压缩炸弹会撒谎。 + fn copy_entry_within_budget( + reader: &mut R, + writer: &mut W, + total_bytes: &mut u64, + ) -> Result<()> { + let mut buffer = [0u8; 16 * 1024]; + loop { + let read = reader.read(&mut buffer)?; + if read == 0 { + return Ok(()); + } + Self::charge_archive_budget(total_bytes, read as u64)?; + writer.write_all(&buffer[..read])?; + } + } + + /// 读取 symlink 条目声明的目标路径。 + /// + /// 这条分支曾是唯一一处不经预算的解压：`read_to_string` 直接把整条解压流吞进 + /// 内存，而 zip 2.4.2 的 `make_reader`（read.rs:437-449）只叠了 CRC 校验， + /// **没有**按声明的 uncompressed_size 截断。于是一个打着 symlink 标志、 + /// deflate 后能膨胀到数 GB 的条目就是一颗内存炸弹，且预算读数全程为 0。 + /// + /// 超长或非 UTF-8 一律返回 `None` 让调用方跳过：合法的 symlink 目标是一条 + /// 路径，这两种形状都不可能是真实数据。 + fn read_symlink_target( + reader: &mut R, + total_bytes: &mut u64, + ) -> Result> { + let mut raw = Vec::new(); + // 多读一个字节，用来区分"正好到上限"和"被截断" + let mut limited = std::io::Read::take(reader, MAX_SYMLINK_TARGET_BYTES + 1); + std::io::Read::read_to_end(&mut limited, &mut raw)?; + if raw.len() as u64 > MAX_SYMLINK_TARGET_BYTES { + return Ok(None); + } + Self::charge_archive_budget(total_bytes, raw.len() as u64)?; + Ok(String::from_utf8(raw) + .ok() + .map(|target| target.trim().to_string())) + } + + /// 建目录并按**实际新建的层数**计费。 + /// + /// `create_dir_all` 会一次性把缺失的父目录全建出来，所以一个条目名 + /// `a/a/…/a/f.txt` 可以隐式造出几百层目录。只在 symlink 物化那条路径上给目录 + /// 计费是不够的：常规解压这条路上，不到 10_000 个条目照样能造出数百万目录， + /// 而内容字节几乎为零。 + fn create_dir_all_within_budget(path: &Path, total_bytes: &mut u64) -> Result<()> { + let missing = path.ancestors().take_while(|p| !p.exists()).count() as u64; + if missing > 0 { + Self::charge_archive_budget(total_bytes, missing * DIRECTORY_BUDGET_COST)?; + } + fs::create_dir_all(path)?; + Ok(()) + } + + /// 归档预算的唯一扣费点。 + /// + /// 抽出来是因为「写文件内容」不是归档能消耗的唯一资源：symlink 物化出来的 + /// 目录一个字节都不写，但每一个都要占 inode 与一个目录块，而第二遍的 + /// symlink 解析可以让目录数量按层数指数增长。只按内容字节计费时，一个全是 + /// 空目录的归档能把预算读数一直停在 0。 + fn charge_archive_budget(total_bytes: &mut u64, amount: u64) -> Result<()> { + if total_bytes.saturating_add(amount) > MAX_ARCHIVE_TOTAL_BYTES { + let limit_mb = (MAX_ARCHIVE_TOTAL_BYTES / 1024 / 1024).to_string(); + return Err(anyhow::anyhow!(format_skill_error( + "ARCHIVE_TOO_LARGE", + &[("limit_mb", &limit_mb)], + Some("checkZipContent"), + ))); + } + *total_bytes += amount; + Ok(()) + } + + /// 把 GitHub 仓库归档解压到 `dest`（剥掉归档自带的一层根目录）。 + /// + /// 与 `download_and_extract` 分离，使 zip-slip 防护可在不联网的情况下被单测覆盖。 + fn extract_repo_archive( + mut archive: zip::ZipArchive, + dest: &Path, + ) -> Result<()> { let root_name = if !archive.is_empty() { let first_file = archive.by_index(0)?; let name = first_file.name(); @@ -2268,48 +2609,111 @@ impl SkillService { ))); }; + // 归档字节完全由第三方控制（仓库可经 deeplink 添加），所以解压必须限量， + // 否则一个几 MB 的压缩炸弹就能塞满磁盘。webdav_sync/archive.rs 早有同款 + // 双重上限，这条下载路径一直没有。 + if archive.len() > MAX_ARCHIVE_ENTRIES { + let count = archive.len().to_string(); + let limit = MAX_ARCHIVE_ENTRIES.to_string(); + return Err(anyhow::anyhow!(format_skill_error( + "ARCHIVE_TOO_MANY_ENTRIES", + &[("count", &count), ("limit", &limit)], + Some("checkZipContent"), + ))); + } + let mut total_bytes: u64 = 0; + // 第一遍：解压普通文件和目录，收集 symlink 条目 let mut symlinks: Vec<(PathBuf, String)> = Vec::new(); for i in 0..archive.len() { let mut file = archive.by_index(i)?; - let file_path = file.name().to_string(); + // 第一道：enclosed_name() 拒绝绝对路径、盘符前缀，以及净深度为负 + // （即逃出归档自身根目录）的条目。skill 仓库可由 deeplink 添加， + // 压缩包内容属第三方可控输入。 + let Some(safe_path) = file.enclosed_name() else { + log::warn!("跳过不安全的压缩包条目: {}", file.name()); + continue; + }; - let relative_path = - if let Some(stripped) = file_path.strip_prefix(&format!("{root_name}/")) { - stripped - } else { - continue; - }; + // GitHub 归档统一带一层 `-/` 根目录，需剥掉后再落盘。 + let Ok(relative_path) = safe_path.strip_prefix(&root_name) else { + continue; + }; - if relative_path.is_empty() { + // 第二道：enclosed_name() 的保证是相对**归档根**的，且它不规范化路径 + // ——`..` 会原样留在返回值里。上面剥掉 root_name 等于花掉一级深度预算， + // 于是 `repo-main/../evil` 这类条目仍能落到 dest 之外（Unix 逃一层； + // Windows 上 root_name 可含反斜杠而被当作多段，逃逸深度随之放大）。 + // 因此 join 之前必须对**实际使用的相对路径**再验一次。 + if relative_path + .components() + .any(|c| matches!(c, Component::ParentDir)) + { + log::warn!("跳过越界的压缩包条目: {}", file.name()); + continue; + } + + if relative_path.as_os_str().is_empty() { continue; } let outpath = dest.join(relative_path); if file.is_symlink() { - // 读取 symlink 目标路径 - let mut target = String::new(); - std::io::Read::read_to_string(&mut file, &mut target)?; - symlinks.push((outpath, target.trim().to_string())); + let Some(target) = Self::read_symlink_target(&mut file, &mut total_bytes)? else { + log::warn!("跳过目标不合法的 symlink 条目: {}", file.name()); + continue; + }; + symlinks.push((outpath, target)); } else if file.is_dir() { - fs::create_dir_all(&outpath)?; + Self::create_dir_all_within_budget(&outpath, &mut total_bytes)?; } else { if let Some(parent) = outpath.parent() { - fs::create_dir_all(parent)?; + Self::create_dir_all_within_budget(parent, &mut total_bytes)?; } let mut outfile = fs::File::create(&outpath)?; - std::io::copy(&mut file, &mut outfile)?; + // 按实际写入的字节累计，而不是信任归档头里声明的 size—— + // 压缩炸弹的声明值可以是假的。 + Self::copy_entry_within_budget(&mut file, &mut outfile, &mut total_bytes)?; } } // 第二遍：解析 symlink，将目标内容复制到 symlink 位置 - Self::resolve_symlinks_in_dir(dest, &symlinks)?; + Self::resolve_symlinks_in_dir(dest, &symlinks, &mut total_bytes)?; Ok(()) } + /// 与 `copy_dir_recursive` 同语义，但把写出的字节计入归档总预算。 + /// 仅用于解压期间物化 symlink——常规的目录复制（安装、备份、迁移）不该受 + /// 归档预算约束，所以两个函数刻意不合并。 + fn copy_dir_within_budget(src: &Path, dest: &Path, total_bytes: &mut u64) -> Result<()> { + Self::create_dir_all_within_budget(dest, total_bytes)?; + + for entry in fs::read_dir(src)? { + let entry = entry?; + let path = entry.path(); + let dest_path = dest.join(entry.file_name()); + + if path.is_dir() { + Self::copy_dir_within_budget(&path, &dest_path, total_bytes)?; + } else { + Self::copy_file_within_budget(&path, &dest_path, total_bytes)?; + } + } + + Ok(()) + } + + /// 复制单个文件并计入归档总预算，复用 `copy_entry_within_budget` 以保证 + /// 上限与报错文案只有一处定义。 + fn copy_file_within_budget(src: &Path, dest: &Path, total_bytes: &mut u64) -> Result<()> { + let mut reader = fs::File::open(src)?; + let mut writer = fs::File::create(dest)?; + Self::copy_entry_within_budget(&mut reader, &mut writer, total_bytes) + } + /// 递归复制目录 fn copy_dir_recursive(src: &Path, dest: &Path) -> Result<()> { fs::create_dir_all(dest)?; @@ -2330,7 +2734,11 @@ impl SkillService { } fn resolve_uninstall_backup_source(skill: &InstalledSkill) -> Result> { - let ssot_path = Self::get_ssot_dir()?.join(&skill.directory); + // 返回值会被整目录复制进 ~/.cc-switch/skill-backups/ 并由 get_skill_backups + // 在界面上列出——脏 directory 在这里等于任意文件读取 + 外泄通道。 + let directory = Self::require_valid_directory(&skill.directory)?; + + let ssot_path = Self::get_ssot_dir()?.join(&directory); if ssot_path.is_dir() { return Ok(Some(ssot_path)); } @@ -2340,7 +2748,7 @@ impl SkillService { Ok(dir) => dir, Err(_) => continue, }; - let candidate = app_dir.join(&skill.directory); + let candidate = app_dir.join(&directory); if candidate.is_dir() { return Ok(Some(candidate)); } @@ -2472,7 +2880,11 @@ impl SkillService { /// GitHub ZIP 归档保留了 symlink 元数据，解压时可通过 `is_symlink()` 检测。 /// 此方法将 symlink 解析为实际文件/目录内容（而非创建真实 symlink）， /// 以确保跨平台兼容且 skill 内容自包含。 - fn resolve_symlinks_in_dir(base_dir: &Path, symlinks: &[(PathBuf, String)]) -> Result<()> { + fn resolve_symlinks_in_dir( + base_dir: &Path, + symlinks: &[(PathBuf, String)], + total_bytes: &mut u64, + ) -> Result<()> { // 规范化 base_dir（macOS 上 /tmp → /private/tmp，需保持一致） let canonical_base = base_dir .canonicalize() @@ -2496,7 +2908,7 @@ impl SkillService { } }; - // 安全检查：确保目标在 base_dir 内（防止路径穿越） + // 安全检查一：确保目标在 base_dir 内（防止路径穿越） if !resolved.starts_with(&canonical_base) { log::warn!( "Symlink 目标超出仓库范围，跳过: {} -> {}", @@ -2506,14 +2918,47 @@ impl SkillService { continue; } - // 复制目标内容到 symlink 位置 + // 安全检查二：目标不能包含 link 自身。上面那条防的是「跑出 base」， + // 防不住「套进自己」——`dir/link -> ..` 解析后正是 base 本身，完全 + // 合规，随后递归复制会把归档根复制进自己的子目录；每递归一层都重新 + // 看到刚落盘的副本，目录树逐层膨胀直到 PATH_MAX 才失败。 + // + // 比较必须在**规范形式**上做：`enclosed_name()` 不规范化路径，只保证 + // 净深度非负，所以 link_path 里可能带着未消解的 `..`（`e/../d/self`）。 + // 拿它按字面跟 canonicalize 过的 resolved 比组件，第一段就会错开 + // （`e` vs `d`），检查形同虚设。link_path 自身此刻尚未落盘，但它的父 + // 目录一定存在——`resolved` 能 canonicalize 成功就蕴含了这一点。 + let canonical_link = match parent.canonicalize() { + Ok(canonical_parent) => match link_path.file_name() { + Some(name) => canonical_parent.join(name), + None => canonical_parent, + }, + // 父目录都不存在时退回字面形式：此时 resolved 多半也解析不出来， + // 上面就已经 continue 了；留着只是不让守卫在意外形状上 panic。 + Err(_) => match link_path.strip_prefix(base_dir) { + Ok(relative) => canonical_base.join(relative), + Err(_) => link_path.clone(), + }, + }; + if canonical_link.starts_with(&resolved) { + log::warn!( + "Symlink 目标包含链接自身，跳过（会导致递归自复制）: {} -> {}", + link_path.display(), + resolved.display() + ); + continue; + } + + // 复制目标内容到 symlink 位置。必须与解压循环共用同一个字节预算： + // 物化走的是这条独立路径，不计费的话「一个大文件 + N 个指向它的 + // symlink」能写下 N 倍字节，而 MAX_ARCHIVE_TOTAL_BYTES 全程显示合规。 if resolved.is_dir() { - Self::copy_dir_recursive(&resolved, link_path)?; + Self::copy_dir_within_budget(&resolved, link_path, total_bytes)?; } else if resolved.is_file() { if let Some(parent) = link_path.parent() { - fs::create_dir_all(parent)?; + Self::create_dir_all_within_budget(parent, total_bytes)?; } - fs::copy(&resolved, link_path)?; + Self::copy_file_within_budget(&resolved, link_path, total_bytes)?; } } Ok(()) @@ -2534,13 +2979,13 @@ impl SkillService { current_app: &AppType, ) -> Result> { // 解压到临时目录 - let temp_dir = Self::extract_local_zip(zip_path)?; + let temp_guard = Self::extract_local_zip(zip_path)?; + let temp_dir = temp_guard.path(); // 扫描所有包含 SKILL.md 的目录 - let skill_dirs = Self::scan_skills_in_dir(&temp_dir)?; + let skill_dirs = Self::scan_skills_in_dir(temp_dir)?; if skill_dirs.is_empty() { - let _ = fs::remove_dir_all(&temp_dir); return Err(anyhow!(format_skill_error( "NO_SKILLS_IN_ZIP", &[], @@ -2574,7 +3019,10 @@ impl SkillService { .map(|s| s.to_string_lossy().to_string()) .unwrap_or_default(); - if skill_dir == temp_dir || dir_name.is_empty() || dir_name.starts_with('.') { + if skill_dir.as_path() == temp_dir + || dir_name.is_empty() + || dir_name.starts_with('.') + { // SKILL.md 在根目录：优先用元数据 name，否则用 ZIP 文件名 meta.as_ref() .and_then(|m| m.name.as_deref()) @@ -2593,7 +3041,6 @@ impl SkillService { let install_name = match install_name { Some(name) => name, None => { - let _ = fs::remove_dir_all(&temp_dir); return Err(anyhow!(format_skill_error( "INVALID_SKILL_DIRECTORY", &[("zip", &zip_path.display().to_string())], @@ -2664,14 +3111,16 @@ impl SkillService { installed.push(skill); } - // 清理临时目录 - let _ = fs::remove_dir_all(&temp_dir); - Ok(installed) } /// 解压本地 ZIP 文件到临时目录 - fn extract_local_zip(zip_path: &Path) -> Result { + /// + /// 返回 `TempDir` 而不是 `PathBuf`：调用方在解压之后还有扫描、复制、写库、 + /// 同步等一长串 `?`，任何一处提前返回都会把最多 512 MiB 的临时内容永久留在 + /// 磁盘上。守卫交给调用方持有，清理就变成作用域结束时自动发生，不再依赖每条 + /// 出口都记得手写 `remove_dir_all`（实测漏了不止一条）。 + fn extract_local_zip(zip_path: &Path) -> Result { let file = fs::File::open(zip_path) .with_context(|| format!("Failed to open ZIP file: {}", zip_path.display()))?; @@ -2686,11 +3135,25 @@ impl SkillService { ))); } + // 与远端归档同一套上限。本地 ZIP 是用户自选文件（信任度更高），但"用户 + // 被诱导打开一个压缩炸弹"仍是常见路径，且两条解压路径共用同一个物化器。 + if archive.len() > MAX_ARCHIVE_ENTRIES { + let count = archive.len().to_string(); + let limit = MAX_ARCHIVE_ENTRIES.to_string(); + return Err(anyhow!(format_skill_error( + "ARCHIVE_TOO_MANY_ENTRIES", + &[("count", &count), ("limit", &limit)], + Some("checkZipContent"), + ))); + } + + // 守卫持有到解压全部成功为止：中途任何 `?` 都会让它清掉半成品目录。 + // 原来在这里就 keep()，超限或解压出错都会留下永久残留。 let temp_dir = tempfile::tempdir()?; let temp_path = temp_dir.path().to_path_buf(); - let _ = temp_dir.keep(); // Keep the directory, we'll clean up later let mut symlinks: Vec<(PathBuf, String)> = Vec::new(); + let mut total_bytes: u64 = 0; for i in 0..archive.len() { let mut file = archive.by_index(i)?; @@ -2699,27 +3162,42 @@ impl SkillService { None => continue, }; + // `enclosed_name()` 只保证净深度非负，**不消解** `..`。这里没有 + // `strip_prefix` 吃掉深度预算，所以逃不出 temp_path；但留着未消解的 + // 路径会让后面的 symlink 自包含检查失去可比性（`e/../d/self` 与 `d` + // 逐组件比在第一段就错开）。在入口就把它们挡掉，保证落进 symlinks + // 表里的路径都是规范形状。 + if file_path + .components() + .any(|c| matches!(c, Component::ParentDir)) + { + log::warn!("跳过越界的压缩包条目: {}", file.name()); + continue; + } + let outpath = temp_path.join(&file_path); if file.is_symlink() { - let mut target = String::new(); - std::io::Read::read_to_string(&mut file, &mut target)?; - symlinks.push((outpath, target.trim().to_string())); + let Some(target) = Self::read_symlink_target(&mut file, &mut total_bytes)? else { + log::warn!("跳过目标不合法的 symlink 条目: {}", file.name()); + continue; + }; + symlinks.push((outpath, target)); } else if file.is_dir() { - fs::create_dir_all(&outpath)?; + Self::create_dir_all_within_budget(&outpath, &mut total_bytes)?; } else { if let Some(parent) = outpath.parent() { - fs::create_dir_all(parent)?; + Self::create_dir_all_within_budget(parent, &mut total_bytes)?; } let mut outfile = fs::File::create(&outpath)?; - std::io::copy(&mut file, &mut outfile)?; + Self::copy_entry_within_budget(&mut file, &mut outfile, &mut total_bytes)?; } } // 解析 symlink - Self::resolve_symlinks_in_dir(&temp_path, &symlinks)?; + Self::resolve_symlinks_in_dir(&temp_path, &symlinks, &mut total_bytes)?; - Ok(temp_path) + Ok(temp_dir) } /// 递归扫描目录查找包含 SKILL.md 的技能目录 @@ -2825,8 +3303,13 @@ impl SkillService { return None; } let (owner, repo) = (parts[0].to_string(), parts[1].to_string()); - // 过滤非 GitHub 来源（如 "skills.volces.com"、"mcp-hub.momenta.works"） - if owner.contains('.') || repo.contains('.') { + // 用与 download_repo 同一套坐标校验，而不是就地写启发式：下面这个 + // readme_url 最终交给 openExternal 打开，是和 build_skill_doc_url + // 同一个 sink。原来的 `contains('.')` 既漏（`splitn(2, '/')` 允许 + // repo 里带 `/`，`owner/a/b` 能拼出三段路径），又误伤（GitHub 仓库 + // 名合法含点）。校验 owner 同时也保留了"过滤非 GitHub 来源"的效果 + // ——`skills.volces.com` 这类带点的 owner 本来就不是合法用户名。 + if Self::validate_repo_ref(&owner, &repo, "main").is_err() { return None; } Some(SkillsShDiscoverableSkill { @@ -2872,12 +3355,8 @@ fn build_repo_info_from_lock( // 优先使用 lock 文件中的 skillPath，否则回退到 dir_name/SKILL.md let fallback = format!("{dir_name}/SKILL.md"); let doc_path = info.skill_path.as_deref().unwrap_or(&fallback); - let url = Some(SkillService::build_skill_doc_url( - &info.owner, - &info.repo, - &url_branch, - doc_path, - )); + let url = + SkillService::build_skill_doc_url(&info.owner, &info.repo, &url_branch, doc_path); ( format!("{}/{}:{dir_name}", info.owner, info.repo), Some(info.owner.clone()), @@ -2915,6 +3394,23 @@ fn save_repos_from_lock( branch: info.branch.clone().unwrap_or_else(|| "HEAD".to_string()), enabled: true, }; + // lock 文件由外部 agents CLI 写入，owner/repo/branch 均未经校验， + // 且 branch 是从 `/tree/`、fragment、`?ref=` 里抠出来的裸串。 + if SkillService::validate_repo_ref( + &skill_repo.owner, + &skill_repo.name, + &skill_repo.branch, + ) + .is_err() + { + log::warn!( + "跳过 agents lock 中坐标非法的仓库: {}/{}@{}", + skill_repo.owner, + skill_repo.name, + skill_repo.branch + ); + continue; + } if let Err(e) = db.save_skill_repo(&skill_repo) { log::warn!("保存 skill 仓库 {}/{} 失败: {}", info.owner, info.repo, e); } else { @@ -2951,6 +3447,13 @@ pub fn migrate_skills_to_ssot(db: &Arc) -> Result { if has_snapshot { for row in &snapshot { + // snapshot 存在 settings 表里，而 settings 在同步范围内、可被远端快照 + // 覆盖。下面 discovered 的每个 key 都会被 join 成路径并写回 skills 表， + // 所以脏值必须在进入 discovered 之前就滤掉。 + if SkillService::require_valid_directory(&row.directory).is_err() { + log::warn!("跳过 SSOT 迁移快照中非法的 directory: {:?}", row.directory); + continue; + } if let Ok(app) = row.app_type.parse::() { discovered .entry(row.directory.clone()) @@ -3053,6 +3556,571 @@ mod tests { use super::*; use tempfile::tempdir; + /// 构造一个模拟 GitHub 归档的 ZIP：带一层 `repo-main/` 根目录， + /// 其中掺入用 `../` 逃逸的恶意条目。 + /// + /// 两个恶意条目走的是**不同**的拦截层，缺一不可： + /// - 两级 `../../`：净深度为负，`enclosed_name()` 自己就会拒绝； + /// - 一级 `../`：净深度非负，`enclosed_name()` **放行**且原样保留 `..`， + /// 只有剥掉 root_name 之后的组件校验才能拦住。 + fn build_zip_with_traversal_entry() -> Vec { + use std::io::Write; + use zip::write::SimpleFileOptions; + + let mut buf = Vec::new(); + { + let mut zip = zip::ZipWriter::new(std::io::Cursor::new(&mut buf)); + let opts = SimpleFileOptions::default(); + + // 合法条目：会被正常解压 + zip.start_file("repo-main/SKILL.md", opts).unwrap(); + zip.write_all(b"---\nname: ok\n---\n").unwrap(); + + // 恶意条目 A：被 enclosed_name() 拒绝 + zip.start_file("repo-main/../../escaped.txt", opts).unwrap(); + zip.write_all(b"pwned").unwrap(); + + // 恶意条目 B：能通过 enclosed_name()，靠组件校验拦截 + zip.start_file("repo-main/../escaped-one-level.txt", opts) + .unwrap(); + zip.write_all(b"pwned").unwrap(); + + zip.finish().unwrap(); + } + buf + } + + #[test] + fn validate_repo_ref_accepts_real_world_coordinates() { + // 合法分支名允许 `/`，不能因为防穿越就把它们一起禁掉 + for branch in [ + "main", + "master", + "HEAD", + "feature/new-thing", + "release/v1.2.3", + "fix-123", + "user.name/topic", + ] { + assert!( + SkillService::validate_repo_ref("farion1231", "cc-switch", branch).is_ok(), + "must accept branch: {branch:?}" + ); + } + assert!(SkillService::validate_repo_ref("a", "b.c_d-e", "main").is_ok()); + } + + #[test] + fn validate_repo_ref_accepts_the_empty_branch_sentinel() { + // 空 branch 与 "HEAD" 在 download_repo 里是同一个哨兵：分支候选表跳过 + // 两者，改试 main / master，所以它们从不进 URL。校验若把空串当非法， + // 存量 skill_repos 行（建表默认 'main'，但空串没被禁）会在 download_repo + // 第一行就 INVALID_REPO_REF，整个技能面板列不出东西——前端两处 + // `repo.branch || "main"` 正是照着"空串可用"写的。 + assert!( + SkillService::validate_repo_ref("farion1231", "cc-switch", "").is_ok(), + "the empty-branch sentinel must stay usable" + ); + } + + #[test] + fn validate_repo_ref_rejects_url_hijacking_branches() { + // 这是核心用例：branch 被拼进 archive URL，URL 解析会消解点段， + // 落点会从 /archive/refs/heads/ 改写成攻击者可上传的 release asset。 + for branch in [ + "../../../releases/download/v1/evil", + "..", + "../x", + "a/../../b", + "a/./b", + "..\\..\\releases\\download\\v1\\evil", + "/leading", + "trailing/", + "double//slash", + "with space", + "frag#ment", + "pct%2e%2e", + "ref@{0}", + "seg.lock", + ".hidden/x", + ] { + assert!( + SkillService::validate_repo_ref("owner", "repo", branch).is_err(), + "must reject branch: {branch:?}" + ); + } + for (owner, name) in [ + ("..", "repo"), + ("own/er", "repo"), + ("owner", ".."), + ("owner", "re/po"), + ("owner", "re po"), + ("", "repo"), + ("owner", ""), + ] { + assert!( + SkillService::validate_repo_ref(owner, name, "main").is_err(), + "must reject coordinates: {owner:?}/{name:?}" + ); + } + } + + #[test] + fn assert_github_archive_url_pins_host_and_path() { + let ok = "https://github.com/owner/repo/archive/refs/heads/main.zip"; + assert!(SkillService::assert_github_archive_url(ok, "owner", "repo").is_ok()); + + // 出口断言必须挡住落点被改写到 release asset 的情况 + for bad in [ + "https://github.com/owner/repo/releases/download/v1/evil.zip", + "https://evil.example/owner/repo/archive/refs/heads/main.zip", + "http://github.com/owner/repo/archive/refs/heads/main.zip", + "https://github.com/other/repo/archive/refs/heads/main.zip", + ] { + assert!( + SkillService::assert_github_archive_url(bad, "owner", "repo").is_err(), + "must reject url: {bad}" + ); + } + } + + #[test] + fn build_skill_doc_url_drops_illegal_coordinates() { + assert_eq!( + SkillService::build_skill_doc_url("owner", "repo", "main", "a/SKILL.md").as_deref(), + Some("https://github.com/owner/repo/blob/main/a/SKILL.md") + ); + // readme_url 会被前端 openExternal 直接打开，非法坐标不得产出链接 + assert!( + SkillService::build_skill_doc_url("owner", "repo", "../../../issues", "x").is_none() + ); + } + + #[test] + fn copy_entry_within_budget_stops_before_exceeding_the_limit() { + // 预算逐块累加，超限时中止且不再继续写——压缩炸弹声明的 size 不可信， + // 所以判断只能基于实际读到的字节。 + let mut total = MAX_ARCHIVE_TOTAL_BYTES - 8; + let mut reader = std::io::Cursor::new(vec![7u8; 64]); + let mut writer: Vec = Vec::new(); + + let err = SkillService::copy_entry_within_budget(&mut reader, &mut writer, &mut total) + .expect_err("must reject once the budget is exhausted"); + assert!( + err.to_string().contains("ARCHIVE_TOO_LARGE"), + "unexpected error: {err}" + ); + assert!( + writer.is_empty(), + "nothing may be written once the chunk would exceed the budget" + ); + + // 预算充足时照常写完 + let mut total = 0u64; + let mut reader = std::io::Cursor::new(vec![7u8; 64]); + let mut writer: Vec = Vec::new(); + SkillService::copy_entry_within_budget(&mut reader, &mut writer, &mut total) + .expect("within budget"); + assert_eq!(writer.len(), 64); + assert_eq!(total, 64); + } + + #[test] + fn extract_repo_archive_rejects_too_many_entries() { + use std::io::Write; + use zip::write::SimpleFileOptions; + + let mut buf = Vec::new(); + { + let mut zip = zip::ZipWriter::new(std::io::Cursor::new(&mut buf)); + let opts = SimpleFileOptions::default(); + for i in 0..(MAX_ARCHIVE_ENTRIES + 1) { + zip.start_file(format!("repo-main/f{i}"), opts).unwrap(); + zip.write_all(b"x").unwrap(); + } + zip.finish().unwrap(); + } + + let temp = tempdir().expect("tempdir"); + let archive = zip::ZipArchive::new(std::io::Cursor::new(buf)).expect("archive parses"); + let err = SkillService::extract_repo_archive(archive, temp.path()) + .expect_err("entry count over the limit must be rejected"); + assert!( + err.to_string().contains("ARCHIVE_TOO_MANY_ENTRIES"), + "unexpected error: {err}" + ); + } + + #[test] + fn extract_repo_archive_rejects_path_traversal_entries() { + let temp = tempdir().expect("tempdir"); + // dest 放在深一层，这样逃逸一层/两层都落在 temp 内、可被检出 + let dest = temp.path().join("nested").join("dest"); + fs::create_dir_all(&dest).expect("create dest"); + + let bytes = build_zip_with_traversal_entry(); + let archive = zip::ZipArchive::new(std::io::Cursor::new(bytes)).expect("archive parses"); + + SkillService::extract_repo_archive(archive, &dest).expect("extract must not fail"); + + // 合法条目正常落盘 + assert!( + dest.join("SKILL.md").is_file(), + "legitimate entry should be extracted" + ); + // 两级逃逸：不得写到 dest 之外 + assert!( + !temp.path().join("escaped.txt").exists(), + "zip-slip entry must not escape dest (temp root)" + ); + assert!( + !temp.path().join("nested").join("escaped.txt").exists(), + "zip-slip entry must not escape dest (parent dir)" + ); + // 一级逃逸：enclosed_name() 放行的那一类，必须被组件校验拦住 + assert!( + !temp + .path() + .join("nested") + .join("escaped-one-level.txt") + .exists(), + "single-`..` entry must not escape dest (enclosed_name allows it)" + ); + } + + #[test] + fn extract_repo_archive_skips_a_symlink_that_contains_itself() { + // `dir/link -> ..` 解析后正是归档根：它**通过**「目标必须在 base 内」的 + // 检查，因为目标就是 base 本身。没有第二道自包含检查时， + // copy_dir_recursive(base, base/dir/link) 会把根复制进自己的子目录， + // 每递归一层都重新看到刚落盘的副本，直到 PATH_MAX 才以 IO 错误收场。 + use std::io::Write; + use zip::write::SimpleFileOptions; + + let mut buf = Vec::new(); + { + let mut zip = zip::ZipWriter::new(std::io::Cursor::new(&mut buf)); + let opts = SimpleFileOptions::default(); + zip.start_file("repo-main/SKILL.md", opts).unwrap(); + zip.write_all(b"---\nname: t\ndescription: d\n---\n") + .unwrap(); + zip.add_directory("repo-main/dir/", opts).unwrap(); + zip.add_symlink("repo-main/dir/link", "..", opts).unwrap(); + zip.finish().unwrap(); + } + + let temp = tempdir().expect("tempdir"); + let dest = temp.path().join("dest"); + fs::create_dir_all(&dest).expect("create dest"); + let archive = zip::ZipArchive::new(std::io::Cursor::new(buf)).expect("archive parses"); + + SkillService::extract_repo_archive(archive, &dest) + .expect("a self-containing symlink must be skipped, not blow up the extraction"); + + assert!( + dest.join("SKILL.md").is_file(), + "legitimate entries must still be extracted" + ); + assert!( + !dest.join("dir").join("link").exists(), + "a symlink whose target contains the link itself must not be materialized" + ); + } + + #[test] + fn symlink_materialization_is_charged_to_the_archive_budget() { + // symlink 的物化走第二遍、与解压循环不同的代码路径。若它不计入同一个 + // 预算，「一个大文件 + N 个指向它的 symlink」就能写下 N 倍字节而上限 + // 全程显示合规。这里把预算预置到接近上限来验证物化确实在计费。 + let temp = tempdir().expect("tempdir"); + let base = temp.path().join("base"); + fs::create_dir_all(base.join("payload")).expect("create payload dir"); + fs::write(base.join("payload").join("big.bin"), vec![b'x'; 4096]).expect("write payload"); + + let symlinks = vec![(base.join("copy"), "payload".to_string())]; + let mut total_bytes = MAX_ARCHIVE_TOTAL_BYTES - 1024; + + let err = SkillService::resolve_symlinks_in_dir(&base, &symlinks, &mut total_bytes) + .expect_err("materializing 4 KiB with 1 KiB of budget left must fail"); + assert!( + err.to_string().contains("ARCHIVE_TOO_LARGE"), + "unexpected error: {err}" + ); + } + + #[test] + fn symlink_guard_sees_through_unnormalized_link_paths() { + // `enclosed_name()` 不消解 `..`，所以 link_path 可能长成 `e/../d/self`。 + // 守卫若拿它按字面跟规范化过的目标比组件，会在第一段（`e` vs `d`）就判定 + // "不包含"——而这个位置物理上就在 `d` 里面，把 `d` 复制进去正是递归自复制。 + let temp = tempdir().expect("tempdir"); + let base = temp.path().join("base"); + fs::create_dir_all(base.join("d").join("sub")).expect("create d"); + fs::create_dir_all(base.join("e")).expect("create e"); + + let link_path = base.join("e").join("..").join("d").join("self"); + let symlinks = vec![(link_path, ".".to_string())]; + let mut total_bytes = 0u64; + + SkillService::resolve_symlinks_in_dir(&base, &symlinks, &mut total_bytes) + .expect("a self-containing symlink must be skipped, not blow up the extraction"); + + assert!( + !base.join("d").join("self").exists(), + "a link that physically lives inside its own target must not be materialized" + ); + } + + /// 记录实际被消耗了多少字节的 reader。 + /// + /// 直接断言返回值是不够的：函数末尾本就有一道长度检查，把 `take` 的上限拆掉 + /// 之后它照样返回 `None`，断言仍然通过——而炸弹的危害全在读取过程里，不在 + /// 返回值。只有观测消耗量才能真正钉住"读取是有界的"。 + struct CountingReader { + remaining: u64, + consumed: u64, + } + + impl std::io::Read for CountingReader { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + if self.remaining == 0 { + return Ok(0); + } + let n = buf.len().min(self.remaining as usize); + buf[..n].fill(b'a'); + self.remaining -= n as u64; + self.consumed += n as u64; + Ok(n) + } + } + + #[test] + fn read_symlink_target_is_bounded_and_charged() { + // 一个打着 symlink 标志、解压流却极大的条目：zip 2.4.2 的 make_reader + // 不按声明的 uncompressed_size 截断，没有上限就会被整条读进内存。 + let mut oversized = CountingReader { + remaining: 8 * 1024 * 1024, + consumed: 0, + }; + let mut total_bytes = 0u64; + + let target = SkillService::read_symlink_target(&mut oversized, &mut total_bytes) + .expect("an oversized target must be skipped, not raise"); + assert!( + target.is_none(), + "a target longer than a path can plausibly be must be rejected" + ); + assert_eq!(total_bytes, 0, "a rejected target must not be charged"); + assert!( + oversized.consumed <= MAX_SYMLINK_TARGET_BYTES + 1, + "the read must stop at the cap instead of draining the stream, consumed {}", + oversized.consumed + ); + + // 正常目标照常读出来并计费 + let mut normal = std::io::Cursor::new(b"../shared".to_vec()); + let target = SkillService::read_symlink_target(&mut normal, &mut total_bytes) + .expect("a normal target must be read"); + assert_eq!(target.as_deref(), Some("../shared")); + assert_eq!(total_bytes, 9); + } + + #[test] + fn directory_materialization_is_charged_to_the_archive_budget() { + // 全是空目录的归档一个内容字节都不写。不给目录计费，第二遍的 symlink + // 解析就能让目录数按层数指数增长，而预算读数一直停在 0。 + let temp = tempdir().expect("tempdir"); + let src = temp.path().join("src"); + fs::create_dir_all(src.join("a").join("b")).expect("create tree"); + + let mut total_bytes = MAX_ARCHIVE_TOTAL_BYTES - DIRECTORY_BUDGET_COST; + let err = + SkillService::copy_dir_within_budget(&src, &temp.path().join("dest"), &mut total_bytes) + .expect_err("materializing directories past the limit must fail"); + assert!( + err.to_string().contains("ARCHIVE_TOO_LARGE"), + "unexpected error: {err}" + ); + } + + #[test] + fn create_dir_all_charges_every_directory_it_creates() { + // `create_dir_all` 一次能把缺失的父目录全建出来，所以一个条目名 + // `a/a/…/a/f.txt` 可以隐式造出几百层。按调用次数计费会严重低估。 + let temp = tempdir().expect("tempdir"); + let deep = temp.path().join("a").join("b").join("c"); + + // 预算只够两层，建三层必须被拦下 + let mut total_bytes = MAX_ARCHIVE_TOTAL_BYTES - 2 * DIRECTORY_BUDGET_COST; + let err = SkillService::create_dir_all_within_budget(&deep, &mut total_bytes) + .expect_err("creating more directories than the budget allows must fail"); + assert!( + err.to_string().contains("ARCHIVE_TOO_LARGE"), + "unexpected error: {err}" + ); + assert!( + !deep.exists(), + "nothing must be created once the budget is exceeded" + ); + } + + #[test] + #[serial_test::serial] + fn extract_local_zip_leaves_no_partial_directory_when_it_fails() { + use std::io::Write; + use zip::write::SimpleFileOptions; + + // TMPDIR 要在建这两个目录之后再改，否则它们自己就落进被观测的目录里 + let holder = tempdir().expect("tempdir"); + let scratch = tempdir().expect("tempdir"); + + let mut buf = Vec::new(); + { + let mut zip = zip::ZipWriter::new(std::io::Cursor::new(&mut buf)); + let opts = SimpleFileOptions::default(); + // `x` 先落成文件，再要求把它当目录用 —— create_dir_all 必然失败， + // 而这个失败发生在临时目录已经建好、且已经写进去东西之后 + zip.start_file("x", opts).unwrap(); + zip.write_all(b"i am a file").unwrap(); + zip.start_file("x/y", opts).unwrap(); + zip.write_all(b"and my parent is not a directory").unwrap(); + zip.finish().unwrap(); + } + let zip_path = holder.path().join("collide.zip"); + fs::write(&zip_path, &buf).expect("write zip"); + + let original = std::env::var_os("TMPDIR"); + std::env::set_var("TMPDIR", scratch.path()); + let result = SkillService::extract_local_zip(&zip_path); + match original { + Some(value) => std::env::set_var("TMPDIR", value), + None => std::env::remove_var("TMPDIR"), + } + + assert!( + result.is_err(), + "the fixture must actually fail after the temp dir exists" + ); + let leftovers: Vec<_> = fs::read_dir(scratch.path()) + .expect("read scratch") + .filter_map(Result::ok) + .map(|entry| entry.path()) + .collect(); + assert!( + leftovers.is_empty(), + "a failed extraction must not leave a partial directory behind: {leftovers:?}" + ); + } + + #[test] + #[serial_test::serial] + fn extract_local_zip_hands_back_a_guard_that_owns_the_tree() { + use std::io::Write; + use zip::write::SimpleFileOptions; + + // 解压成功之后调用方还要走扫描 / 取 SSOT / 复制 / 写库 / 同步一长串 `?`。 + // 之前返回裸 PathBuf，清理靠每条出口手写 remove_dir_all——实测漏了不止一条 + // （install_from_zip 的 copy_dir_recursive 与 save_skill、update_skill 的 + // copy_dir_recursive、fetch_repo_skills 的 scan_dir_recursive 都会漏）。 + let holder = tempdir().expect("tempdir"); + let scratch = tempdir().expect("tempdir"); + + let mut buf = Vec::new(); + { + let mut zip = zip::ZipWriter::new(std::io::Cursor::new(&mut buf)); + let opts = SimpleFileOptions::default(); + zip.start_file("s/SKILL.md", opts).unwrap(); + zip.write_all(b"# skill").unwrap(); + zip.finish().unwrap(); + } + let zip_path = holder.path().join("ok.zip"); + fs::write(&zip_path, &buf).expect("write zip"); + + let original = std::env::var_os("TMPDIR"); + std::env::set_var("TMPDIR", scratch.path()); + let extracted = SkillService::extract_local_zip(&zip_path); + match original { + Some(value) => std::env::set_var("TMPDIR", value), + None => std::env::remove_var("TMPDIR"), + } + + let extracted = extracted.expect("extract must succeed"); + assert!( + extracted.path().join("s").join("SKILL.md").exists(), + "the fixture must actually extract something worth cleaning up" + ); + + // 模拟调用方在下游任意一个 `?` 上提前返回 + drop(extracted); + + let leftovers: Vec<_> = fs::read_dir(scratch.path()) + .expect("read scratch") + .filter_map(Result::ok) + .map(|entry| entry.path()) + .collect(); + assert!( + leftovers.is_empty(), + "dropping the extraction result must take the whole tree with it: {leftovers:?}" + ); + } + + #[test] + fn extract_local_zip_rejects_dot_dot_entries() { + use std::io::Write; + use zip::write::SimpleFileOptions; + + let mut buf = Vec::new(); + { + let mut zip = zip::ZipWriter::new(std::io::Cursor::new(&mut buf)); + let opts = SimpleFileOptions::default(); + zip.add_directory("d/", opts).unwrap(); + zip.add_directory("e/", opts).unwrap(); + // 净深度非负，enclosed_name() 放行；未消解的 `..` 会把它落到 d 里面 + zip.start_file("e/../d/leaked.txt", opts).unwrap(); + zip.write_all(b"x").unwrap(); + zip.finish().unwrap(); + } + + let temp = tempdir().expect("tempdir"); + let zip_path = temp.path().join("dots.zip"); + fs::write(&zip_path, &buf).expect("write zip"); + + let extracted = SkillService::extract_local_zip(&zip_path).expect("extract must not fail"); + assert!( + !extracted.path().join("d").join("leaked.txt").exists(), + "an entry with an unresolved `..` must be skipped, not silently relocated" + ); + } + + #[test] + fn extract_local_zip_rejects_too_many_entries() { + // 本地 ZIP 走的是另一个解压器，条目上限曾只加在远端归档那条路径上。 + use std::io::Write; + use zip::write::SimpleFileOptions; + + let mut buf = Vec::new(); + { + let mut zip = zip::ZipWriter::new(std::io::Cursor::new(&mut buf)); + let opts = SimpleFileOptions::default(); + for i in 0..(MAX_ARCHIVE_ENTRIES + 1) { + zip.start_file(format!("f{i}"), opts).unwrap(); + zip.write_all(b"x").unwrap(); + } + zip.finish().unwrap(); + } + + let temp = tempdir().expect("tempdir"); + let zip_path = temp.path().join("bomb.zip"); + fs::write(&zip_path, &buf).expect("write zip"); + + let err = SkillService::extract_local_zip(&zip_path) + .expect_err("entry count over the limit must be rejected for local ZIPs too"); + assert!( + err.to_string().contains("ARCHIVE_TOO_MANY_ENTRIES"), + "unexpected error: {err}" + ); + } + fn write_skill(dir: &Path, name: &str) { fs::create_dir_all(dir).expect("create skill dir"); fs::write( @@ -3062,6 +4130,250 @@ mod tests { .expect("write SKILL.md"); } + /// CC_SWITCH_TEST_HOME 隔离守卫（serial 测试间互斥由 #[serial] 保证， + /// 守卫只负责在测试结束后恢复原值）。 + struct TestHomeGuard(Option); + impl TestHomeGuard { + fn set(home: &Path) -> Self { + let guard = Self(std::env::var_os("CC_SWITCH_TEST_HOME")); + std::env::set_var("CC_SWITCH_TEST_HOME", home); + guard + } + } + impl Drop for TestHomeGuard { + fn drop(&mut self) { + match self.0.take() { + Some(value) => std::env::set_var("CC_SWITCH_TEST_HOME", value), + None => std::env::remove_var("CC_SWITCH_TEST_HOME"), + } + } + } + + fn poisoned_skill(id: &str, directory: &str) -> InstalledSkill { + InstalledSkill { + id: id.to_string(), + name: "poisoned".to_string(), + description: None, + directory: directory.to_string(), + repo_owner: None, + repo_name: None, + repo_branch: None, + readme_url: None, + apps: SkillApps::default(), + installed_at: 0, + content_hash: None, + updated_at: 0, + } + } + + #[test] + fn require_valid_directory_accepts_single_segment_names_only() { + assert_eq!( + SkillService::require_valid_directory("my-skill").expect("valid name"), + "my-skill" + ); + for bad in [ + "..", + "../..", + "../../etc", + "a/b", + "a\\b", + "", + ".hidden", + "C:\\evil", + "/etc", + ] { + assert!( + SkillService::require_valid_directory(bad).is_err(), + "must reject: {bad:?}" + ); + } + } + + #[test] + #[serial_test::serial] + fn restore_from_backup_rejects_traversal_directory_in_metadata() { + let temp = tempdir().expect("tempdir"); + let _guard = TestHomeGuard::set(temp.path()); + + // 手工放置一个备份：meta.json 里的 directory 指向 SSOT 之外。 + // SSOT 位于 {home}/.cc-switch/skills，"../../pwned-restore" 若生效会写到 {home}/pwned-restore。 + let backup_id = "20260727_120000_evil"; + let backup_dir = SkillService::get_backup_dir() + .expect("backup dir") + .join(backup_id); + write_skill(&backup_dir.join("skill"), "evil"); + let metadata = SkillBackupMetadata { + skill: poisoned_skill("owner/repo:evil", "../../pwned-restore"), + backup_created_at: 0, + source_path: "x".to_string(), + }; + fs::write( + backup_dir.join("meta.json"), + serde_json::to_string_pretty(&metadata).expect("serialize metadata"), + ) + .expect("write meta.json"); + + let db = std::sync::Arc::new(Database::memory().expect("memory db")); + let result = SkillService::restore_from_backup(&db, backup_id, &AppType::Claude); + + assert!( + result.is_err(), + "restore must reject a traversal directory from meta.json" + ); + assert!( + !temp.path().join("pwned-restore").exists(), + "restore must not write outside the SSOT dir" + ); + } + + #[test] + #[serial_test::serial] + fn remove_from_app_rejects_traversal_directory() { + let temp = tempdir().expect("tempdir"); + let _guard = TestHomeGuard::set(temp.path()); + + // 受害目录与 app skills 目录都先建好，保证未修复时代码真的能删到它： + // app_dir = {home}/.claude/skills，"../../victim-remove" 解析为 {home}/victim-remove。 + let victim = temp.path().join("victim-remove"); + fs::create_dir_all(&victim).expect("create victim dir"); + fs::create_dir_all(temp.path().join(".claude").join("skills")).expect("create app dir"); + + let result = SkillService::remove_from_app("../../victim-remove", &AppType::Claude); + + assert!(result.is_err(), "remove_from_app must reject traversal"); + assert!(victim.exists(), "victim directory must not be deleted"); + } + + #[test] + #[serial_test::serial] + fn uninstall_rejects_traversal_directory_from_db_row() { + let temp = tempdir().expect("tempdir"); + let _guard = TestHomeGuard::set(temp.path()); + + // 模拟同步导入灌进来的脏数据：directory 含路径穿越（save_skill 不校验， + // 与 import_sql_string_for_sync 的效果一致）。SSOT = {home}/.cc-switch/skills， + // "../../victim-uninstall" 解析为 {home}/victim-uninstall。 + let victim = temp.path().join("victim-uninstall"); + fs::create_dir_all(&victim).expect("create victim dir"); + + let db = std::sync::Arc::new(Database::memory().expect("memory db")); + let skill = poisoned_skill("owner/repo:evil", "../../victim-uninstall"); + db.save_skill(&skill).expect("seed poisoned row"); + + let result = SkillService::uninstall(&db, &skill.id); + + // 危险的文件系统操作必须被跳过…… + assert!(victim.exists(), "victim directory must not be deleted"); + // ……但记录本身必须能删掉。db.delete_skill 全项目只有 uninstall 一处调用 + // 且未暴露为命令，若这里返回 Err，脏行就永远无法从界面清除。 + assert!( + result.is_ok(), + "uninstall must still succeed so the poisoned row can be removed: {result:?}" + ); + assert!( + db.get_installed_skill(&skill.id) + .expect("query skill") + .is_none(), + "poisoned row must be deleted from the database" + ); + } + + #[test] + #[serial_test::serial] + fn migrate_storage_skips_bad_rows_without_moving_foreign_dirs() { + /// skill_storage_location 存在进程级全局 settings_store 里，migrate_storage + /// 成功后会改写它。#[serial] 只保证互斥、不负责还原，必须自己复位， + /// 否则后续测试的 get_ssot_dir() 会解析到另一个位置。 + struct StorageLocationGuard(SkillStorageLocation); + impl Drop for StorageLocationGuard { + fn drop(&mut self) { + let _ = crate::settings::set_skill_storage_location(self.0); + } + } + let _location_guard = StorageLocationGuard(crate::settings::get_skill_storage_location()); + + let temp = tempdir().expect("tempdir"); + let _guard = TestHomeGuard::set(temp.path()); + + // migrate_storage 会 fs::rename / remove_dir_all，脏 directory 能把 + // SSOT 之外的任意目录搬走或删掉。 + let victim = temp.path().join("victim-migrate"); + fs::create_dir_all(&victim).expect("create victim dir"); + + let db = std::sync::Arc::new(Database::memory().expect("memory db")); + let skill = poisoned_skill("owner/repo:evil", "../../victim-migrate"); + db.save_skill(&skill).expect("seed poisoned row"); + + // 必须迁到与当前不同的位置，否则函数在 current == target 处直接短路返回 + let result = SkillService::migrate_storage(&db, SkillStorageLocation::Unified) + .expect("migration must not abort"); + + assert!(victim.exists(), "foreign directory must not be moved away"); + assert_eq!( + result.migrated_count, 0, + "poisoned row must not count as migrated" + ); + assert!( + !result.errors.is_empty(), + "the skipped row must be reported through the errors channel" + ); + } + + #[test] + #[serial_test::serial] + fn uninstall_backup_source_rejects_traversal_directory() { + let temp = tempdir().expect("tempdir"); + let _guard = TestHomeGuard::set(temp.path()); + + // 备份源会被整目录复制进 skill-backups 并在界面列出 → 任意文件外泄面。 + let secrets = temp.path().join("secrets"); + fs::create_dir_all(&secrets).expect("create secrets dir"); + fs::write(secrets.join("id_rsa"), b"PRIVATE").expect("write secret"); + + let skill = poisoned_skill("owner/repo:evil", "../../secrets"); + let result = SkillService::resolve_uninstall_backup_source(&skill); + + assert!( + result.is_err(), + "backup source must reject a traversal directory" + ); + } + + #[test] + #[serial_test::serial] + fn sync_to_app_skips_bad_rows_instead_of_aborting_the_whole_app() { + let temp = tempdir().expect("tempdir"); + let _guard = TestHomeGuard::set(temp.path()); + + let ssot_dir = SkillService::get_ssot_dir().expect("ssot dir"); + write_skill(&ssot_dir.join("good-skill"), "good"); + + let db = std::sync::Arc::new(Database::memory().expect("memory db")); + // 一条脏行 + 一条正常行。脏行来自同步导入/存量数据，不得连累正常行—— + // sync_to_app 在切换供应商时触发，整体中断会让所有 skill 一起失效。 + // + // 名字刻意让脏行排前面：查询是 `ORDER BY name ASC`，脏行必须先被处理， + // 否则「未修复时会中断」这个前提不成立，测试就成了摆设。 + let mut bad = poisoned_skill("owner/repo:bad", "../../escape-sync"); + bad.name = "a-poisoned".to_string(); + bad.apps = SkillApps::only(&AppType::Claude); + db.save_skill(&bad).expect("seed poisoned row"); + + let mut good = poisoned_skill("owner/repo:good", "good-skill"); + good.name = "z-healthy".to_string(); + good.apps = SkillApps::only(&AppType::Claude); + db.save_skill(&good).expect("seed good row"); + + SkillService::sync_to_app(&db, &AppType::Claude).expect("sync must not abort"); + + let app_dir = SkillService::get_app_skills_dir(&AppType::Claude).expect("app dir"); + assert!( + app_dir.join("good-skill").exists(), + "the healthy skill must still be synced despite the poisoned row" + ); + } + #[test] // serial：与 backup/s3_sync/deeplink 等同样读写进程级 CC_SWITCH_TEST_HOME 的测试互斥， // EnvGuard 只负责恢复不提供互斥。 diff --git a/src/components/providers/forms/hooks/useGeminiCommonConfig.ts b/src/components/providers/forms/hooks/useGeminiCommonConfig.ts index b521bf7fe..76b820b6a 100644 --- a/src/components/providers/forms/hooks/useGeminiCommonConfig.ts +++ b/src/components/providers/forms/hooks/useGeminiCommonConfig.ts @@ -5,11 +5,77 @@ import { configApi } from "@/lib/api"; const LEGACY_STORAGE_KEY = "cc-switch:gemini-common-config-snippet"; const DEFAULT_GEMINI_COMMON_CONFIG_SNIPPET = "{}"; +/** 供应商专属、不可共享的键（端点与主凭据），按名单剥离。 */ const GEMINI_COMMON_ENV_FORBIDDEN_KEYS = [ "GOOGLE_GEMINI_BASE_URL", "GEMINI_API_KEY", ] as const; -type GeminiForbiddenEnvKey = (typeof GEMINI_COMMON_ENV_FORBIDDEN_KEYS)[number]; + +/** + * 凭据键的模式匹配，对应后端 `ProviderService::is_sensitive_config_key`。 + * + * 共享片段会被深合并进**其它** Gemini 供应商的 env，所以任何凭据都不能进来。 + * 固定名单挡不住下一个 `*_API_KEY`（`GOOGLE_API_KEY` 就是这么漏过去的）， + * 必须用模式覆盖整类。两端判定要一致，否则后端清理完还能从这里手工写回去。 + */ +const SENSITIVE_EXACT = [ + "APIKEY", + "API_KEY", + "TOKEN", + "SECRET", + "PASSWORD", + "CREDENTIALS", +]; +// 单数 `_TOKEN` 命中 AWS_SESSION_TOKEN 等，但不误伤复数 `_TOKENS`（那是正常配置） +const SENSITIVE_SUFFIXES = [ + // 裸 `_KEY` 是最常见的凭据写法（OPENAI_KEY / GROQ_KEY / XAI_KEY…），必须单列： + // 只枚举 `_API_KEY` / `_ACCESS_KEY` 这些子类，等于把最普通的那一种漏在外面。 + "_KEY", + "_API_KEY", + "_ACCESS_KEY", + "_ACCESS_KEY_ID", + "_KEY_ID", + "_PRIVATE_KEY", + // 不带分隔符的复合写法各走各的后缀：`_KEY` 够不着 `..._APIKEY`。 + "_APIKEY", + "_ACCESSKEY", + "_SECRETKEY", + "_APITOKEN", + "_AUTH_TOKEN", + "_TOKEN", + // GITHUB_PAT / GITLAB_PAT 等 personal access token 的惯用写法， + // 既不含 TOKEN 也不含 KEY，前面每一条规则都够不着。 + "_PAT", + // 口令类的常见缩写。`_PASS` 不会误伤 `*_BYPASS`，`_PWD` 不会误伤 PWD / OLDPWD。 + "_PWD", + "_PASS", + "_PASSPHRASE", + "_CREDS", +]; +const SENSITIVE_CONTAINS = [ + "SECRET", + "PASSWORD", + "PASSWD", + "CREDENTIAL", + "PRIVATE_KEY", + "BEARER_TOKEN", +]; + +function isForbiddenCommonEnvKey(name: string): boolean { + if ( + GEMINI_COMMON_ENV_FORBIDDEN_KEYS.includes( + name as (typeof GEMINI_COMMON_ENV_FORBIDDEN_KEYS)[number], + ) + ) { + return true; + } + const upper = name.toUpperCase(); + return ( + SENSITIVE_EXACT.includes(upper) || + SENSITIVE_SUFFIXES.some((suffix) => upper.endsWith(suffix)) || + SENSITIVE_CONTAINS.some((part) => upper.includes(part)) + ); +} interface UseGeminiCommonConfigProps { envValue: string; @@ -90,9 +156,7 @@ export function useGeminiCommonConfig({ } const keys = Object.keys(parsed); - const forbiddenKeys = keys.filter((key) => - GEMINI_COMMON_ENV_FORBIDDEN_KEYS.includes(key as GeminiForbiddenEnvKey), - ); + const forbiddenKeys = keys.filter(isForbiddenCommonEnvKey); if (forbiddenKeys.length > 0) { return { env: {}, diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index c2c745141..6e682ab8a 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -2283,6 +2283,9 @@ "skillDirNotFound": "Skill directory not found: {{path}}", "directoryConflict": "Skill directory '{{directory}}' is already occupied by {{existing_repo}}, cannot install from {{new_repo}}", "emptyArchive": "Downloaded archive is empty", + "invalidRepoRef": "Invalid repository reference: {{owner}}/{{name}}", + "archiveTooLarge": "Archive exceeds the {{limit_mb}} MB extraction limit; aborted", + "archiveTooManyEntries": "Archive has too many entries ({{count}}); the limit is {{limit}}", "downloadFailed": "Download failed: HTTP {{status}}", "allBranchesFailed": "All branches failed, tried: {{branches}}", "httpError": "HTTP error {{status}}", diff --git a/src/i18n/locales/ja.json b/src/i18n/locales/ja.json index f9750103d..af05d7f15 100644 --- a/src/i18n/locales/ja.json +++ b/src/i18n/locales/ja.json @@ -2283,6 +2283,9 @@ "skillDirNotFound": "スキルディレクトリが見つかりません: {{path}}", "directoryConflict": "スキルディレクトリ '{{directory}}' は既に {{existing_repo}} で使用されています。{{new_repo}} からインストールできません", "emptyArchive": "ダウンロードしたアーカイブが空です", + "invalidRepoRef": "リポジトリの指定が不正です：{{owner}}/{{name}}", + "archiveTooLarge": "アーカイブが展開上限 {{limit_mb}} MB を超えたため中止しました", + "archiveTooManyEntries": "アーカイブのエントリ数が多すぎます（{{count}}）。上限は {{limit}} です", "downloadFailed": "ダウンロードに失敗しました: HTTP {{status}}", "allBranchesFailed": "すべてのブランチで失敗しました。試行: {{branches}}", "httpError": "HTTP エラー {{status}}", diff --git a/src/i18n/locales/zh-TW.json b/src/i18n/locales/zh-TW.json index fecf2be7d..c9673be55 100644 --- a/src/i18n/locales/zh-TW.json +++ b/src/i18n/locales/zh-TW.json @@ -2254,6 +2254,9 @@ "skillDirNotFound": "技能目錄不存在：{{path}}", "directoryConflict": "技能目錄 '{{directory}}' 已被 {{existing_repo}} 佔用，無法從 {{new_repo}} 安裝", "emptyArchive": "下載的壓縮檔為空", + "invalidRepoRef": "倉庫位址不合法：{{owner}}/{{name}}", + "archiveTooLarge": "壓縮檔解壓後超過 {{limit_mb}} MB 上限，已中止", + "archiveTooManyEntries": "壓縮檔項目過多（{{count}}），上限 {{limit}}", "downloadFailed": "下載失敗：HTTP {{status}}", "allBranchesFailed": "所有分支下載失敗，嘗試了：{{branches}}", "httpError": "HTTP 錯誤 {{status}}", diff --git a/src/i18n/locales/zh.json b/src/i18n/locales/zh.json index 826d797cb..550f4a882 100644 --- a/src/i18n/locales/zh.json +++ b/src/i18n/locales/zh.json @@ -2283,6 +2283,9 @@ "skillDirNotFound": "技能目录不存在：{{path}}", "directoryConflict": "技能目录 '{{directory}}' 已被 {{existing_repo}} 占用，无法从 {{new_repo}} 安装", "emptyArchive": "下载的压缩包为空", + "invalidRepoRef": "仓库地址不合法：{{owner}}/{{name}}", + "archiveTooLarge": "压缩包解压后超过 {{limit_mb}} MB 上限，已中止", + "archiveTooManyEntries": "压缩包条目过多（{{count}}），上限 {{limit}}", "downloadFailed": "下载失败：HTTP {{status}}", "allBranchesFailed": "所有分支下载失败，尝试了：{{branches}}", "httpError": "HTTP 错误 {{status}}", diff --git a/src/lib/errors/skillErrorParser.ts b/src/lib/errors/skillErrorParser.ts index 1ff12e7bd..1cf58ffa4 100644 --- a/src/lib/errors/skillErrorParser.ts +++ b/src/lib/errors/skillErrorParser.ts @@ -37,6 +37,9 @@ function getErrorI18nKey(code: string): string { SKILL_DIR_NOT_FOUND: "skills.error.skillDirNotFound", SKILL_DIRECTORY_CONFLICT: "skills.error.directoryConflict", EMPTY_ARCHIVE: "skills.error.emptyArchive", + INVALID_REPO_REF: "skills.error.invalidRepoRef", + ARCHIVE_TOO_LARGE: "skills.error.archiveTooLarge", + ARCHIVE_TOO_MANY_ENTRIES: "skills.error.archiveTooManyEntries", GET_HOME_DIR_FAILED: "skills.error.getHomeDirFailed", NO_SKILLS_IN_ZIP: "skills.error.noSkillsInZip", };