feat: make TLS certificate verification configurable

[wavezhang]

Add insecure_skip_tls_verify configuration option (default false) to allow users to disable TLS certificate verification.

Configuration:

• Config file: insecure_skip_tls_verify = true
• Environment variable: DEEPSEEK_INSECURE_SKIP_TLS_VERIFY=true

Changes:

• Replaced all hardcoded .danger_accept_invalid_certs(true) across the workspace with configurable flags
• Added env override parsing in both deepseek-config and TUI Config
• Added tests for TOML deserialization, env override application, and HTTP client builder behavior

Note: TLS verification is enabled by default. Only disable when connecting to servers with self-signed or untrusted certificates.

Greptile Summary

This PR makes TLS certificate verification configurable via insecure_skip_tls_verify (config file or DEEPSEEK_INSECURE_SKIP_TLS_VERIFY env var, default false), replacing all previous hardcoded danger_accept_invalid_certs(true) calls across the workspace.

• Adds insecure_skip_tls_verify to both ConfigToml (crates/config) and TUI Config, with env-override support, a load-time tracing::warn!, and a visible TUI banner when the flag is enabled.
• Threads the resolved bool through every network client — DeepSeekClient, McpPool, WebhookHookSink, OpenSandboxBackend, all skill-install/sync paths, the self-updater, and each individual web/search/fetch tool.
• One chunk-index miss in the TUI render function leaves last_composer_area and last_composer_content pointing at the preview strip rather than the composer, breaking composer mouse hit-testing unconditionally after this change.

Confidence Score: 4/5

Safe to merge after fixing the chunk-index off-by-one in ui.rs; the TLS plumbing itself is correct across all network paths.

The layout change in render() inserts a banner chunk at index 1 and correctly re-indexes most downstream accesses, but app.viewport.last_composer_area and the area variable in the block immediately below are still set to chunks[3] (now the preview strip) instead of chunks[4] (the composer). last_composer_area drives mouse hit-testing in mouse_ui.rs, so composer click and drag events will be routed incorrectly whenever a pending-input preview is visible — this is a present, reproducible defect introduced by this PR.

crates/tui/src/tui/ui.rs — the two chunks[3] references immediately after the composer render block need to become chunks[4].

Important Files Changed

Filename	Overview
crates/tui/src/tui/ui.rs	Adds TLS warning banner by inserting a new layout chunk; correctly re-indexes most chunk accesses but misses last_composer_area and last_composer_content, breaking composer mouse hit-testing.
crates/config/src/lib.rs	Adds insecure_skip_tls_verify field to ConfigToml, exposes parse_bool publicly, adds resolve_insecure_skip_tls_verify(), and populates EnvRuntimeOverrides; env-override field in EnvRuntimeOverrides is populated but never consumed (already flagged in previous review).
crates/tui/src/config.rs	Adds insecure_skip_tls_verify to TUI Config, applies env override via apply_env_overrides, emits a tracing warning on load, and propagates to merge_config; implementation is correct and well-tested.
crates/tui/src/mcp.rs	Adds skip_verify to McpPool via a builder method and threads it into McpConnection::connect; consistent and correct across all call sites.
crates/tui/src/skills/install.rs	Threads skip_verify through all skill fetch/install/sync paths; each reqwest client builder is updated consistently.
crates/cli/src/update.rs	Threads skip_verify through the self-update download pipeline (release metadata fetch and binary download); change is complete and consistent.
crates/tui/src/tui/app.rs	Adds insecure_tls_active flag to App struct, derived from config at construction time; straightforward and correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Config File\ninsecure_skip_tls_verify"] --> R
    B["Env Var\nDEEPSEEK_INSECURE_SKIP_TLS_VERIFY"] --> R
    R["resolve_insecure_skip_tls_verify()\nenv var wins, then config, default false"]
    R --> E["EngineConfig\n.insecure_skip_tls_verify"]
    R --> C["DeepSeekClient\nbuild_http_client(skip)"]
    R --> M["McpPool\n.with_skip_tls_verify(skip)"]
    R --> W["WebhookHookSink::new(skip)"]
    R --> U["run_update(beta, skip)"]
    R --> S["OpenSandboxBackend::new(skip)"]
    R --> SK["skills/install\ncandidate_urls / fetch_registry / sync_one_skill"]
    E --> TC["ToolContext\n.insecure_skip_tls_verify"]
    TC --> FU["FetchUrlTool"]
    TC --> WS["WebSearchTool"]
    TC --> WR["WebRunTool"]
    TC --> FT["FinanceTool\n⚠ hardcoded false (prev comment)"]
    TC --> IA["ImageAnalyzeTool\n⚠ hardcoded false (prev comment)"]

Loading

_{Reviews (3): Last reviewed commit: "fix: delegate parse_bool_env to codewhal..." | Re-trigger Greptile}

[gemini-code-assist]

Code Review

This pull request implements a new configuration setting, insecure_skip_tls_verify, and a corresponding environment variable, DEEPSEEK_INSECURE_SKIP_TLS_VERIFY, to allow users to bypass TLS certificate verification for outbound HTTPS requests. This change affects multiple modules, including the CLI update mechanism, skill management, MCP connections, and various network-dependent tools. The review feedback identifies several locations where the skip_verify flag is not properly propagated to internal function calls, specifically regarding mirror-based updates and skill registry synchronization. Furthermore, the reviewer suggests centralizing the environment variable parsing logic to eliminate redundancy and ensure consistent boolean evaluation across the codebase.

[gemini-code-assist]

The skip_verify flag is not propagated to release_from_mirror_base_url. If a user has configured a mirror via environment variables and that mirror uses a self-signed certificate, the update check will fail even if TLS verification is disabled.

[gemini-code-assist]

The skip_verify flag obtained from installer_settings is explicitly ignored here (as _skip_verify) and not passed to install::fetch_registry. This will cause skill listing to fail when using a registry with an untrusted certificate, despite the configuration setting.

[gemini-code-assist]

This boolean parsing logic is duplicated from crates/cli/src/lib.rs. It is recommended to use a centralized helper (like the one used in crates/config) to ensure consistent handling of truthy values (e.g., "1", "true", "yes", "on") across the entire application.

[wavezhang]

Thanks for the review. Here is the status of each comment:

1. crates/cli/src/update.rs:303 — release_from_mirror_base_url only constructs the Release struct (URLs) without making any HTTP request. The actual downloads go through download_url(url, skip_verify), so skip_verify is already applied at the network boundary. No change needed here.

2. crates/tui/src/commands/skills.rs — Fixed. skip_verify is now passed to install::fetch_registry.

3. crates/tui/src/skills/install.rs — Fixed. sync_registry now passes skip_verify to fetch_registry, and fetch_registry uses a custom reqwest::Client with .danger_accept_invalid_certs(skip_verify).

4. crates/cli/src/lib.rs — Fixed. Removed the redundant manual env parsing; now uses store.config.resolve_insecure_skip_tls_verify().

5. crates/tui/src/config.rs — Fixed. Replaced inline boolean parsing with the centralized parse_bool_env helper to stay consistent with crates/config.

All fixes have been squashed into the latest commit.

[Hmbown]

Independent review (security-focused, post-greptile fixes)

Secure by default — confirmed. Option<bool>::unwrap_or(false) everywhere; resolve_insecure_skip_tls_verify defaults false; only enabled via opt-in TOML or DEEPSEEK_INSECURE_SKIP_TLS_VERIFY. Propagation looks complete after the fixes: client.rs, mcp.rs, update.rs, hooks, sandbox, vision, finance, web_run/search, fetch_url, config_ui all carry skip_verify. Min TLS 1.2 still enforced.

Gaps worth addressing:

1. Warning is too quiet. Only one tracing::warn! at config load (config.rs:1309). In TUI mode, users with RUST_LOG unset will never see it. Consider an always-visible status-bar/header banner ("TLS verification DISABLED") while the insecure flag is active — current behavior risks silent insecure operation.
2. Global scope only — no per-provider opt-in. A single global flag disables TLS for all outbound HTTP (provider API, MCP, web tools, update channel, hooks). Users wanting to trust one internal self-signed endpoint must weaken every outbound call, including auto-update downloads. Consider per-provider/per-MCP insecure_skip_tls_verify as a follow-up.
3. No audit log. Toggle isn't recorded in any persistent log/telemetry channel — only ephemeral tracing.
4. Test coverage gap. Tests cover TOML+env parsing and client builder success, but no test asserts the warning fires when the flag is true (warning-path coverage).

v0.8.48 conflict (PR #2256). Non-trivial in crates/cli/src/update.rs (fetch_latest_release signature changed for ReleaseChannel::{Stable,Beta} + FetchedRelease) and crates/config/src/lib.rs test EnvGuard struct. Rebase needed after v0.8.48 lands; also the deepseek_secrets -> codewhale_secrets rename will collide.

[wavezhang]

All review comments have been addressed. Here's a summary of what was done:

P1 — FinanceTool & ImageAnalyzeTool ignore insecure_skip_tls_verify ✅

Commit: 4c76aa2a — "fix: thread skip_verify through FinanceTool and ImageAnalyzeTool"

Threaded skip_verify: bool through the builder chain:

• with_web_tools(self, skip_verify) → FinanceTool::new(skip_verify)
• with_vision_tools(self, config, skip_verify) → ImageAnalyzeTool::new(config, skip_verify)
• with_agent_tools(allow_shell, skip_verify) → forwards to with_web_tools
• with_full_agent_surface(..., skip_verify) → forwards to with_agent_tools
• tool_setup.rs reads from self.config.insecure_skip_tls_verify
• subagent/mod.rs reads from runtime.context.insecure_skip_tls_verify

P1 — parse_bool_env duplicates parse_bool ✅

Commit: 3e66f1da — "fix: delegate parse_bool_env to codewhale_config::parse_bool"

Replaced the 5-line manual match with a one-liner: codewhale_config::parse_bool(raw).ok()

Warning is too quiet ✅

Already handled in the original PR. The TUI renders a persistent yellow warning banner ("⚠ TLS certificate verification is DISABLED") between the header and the chat body whenever insecure_skip_tls_verify is active. This is visible regardless of RUST_LOG settings. See crates/tui/src/tui/ui.rs.

Test coverage — warning path ✅

Already handled. insecure_skip_tls_verify_warning_emitted_when_true() in crates/tui/src/config.rs exercises the tracing::warn! branch with a test subscriber.

v0.8.48 conflicts ✅

Rebased onto latest origin/main. All conflicts resolved:

• crates/cli/src/update.rs — merged ReleaseChannel + skip_verify together
• crates/config/src/lib.rs — EnvGuard struct now carries both codewhale_* fields and deepseek_insecure_skip_tls_verify
• crates/tui/src/core/engine.rs — EngineConfig has tools_always_load, prefer_bwrap, and insecure_skip_tls_verify
• Other TUI files — chunk index adjusted for the TLS banner layout

Remaining as design limitations (acknowledged, not blocking)

• Global scope only — per-provider/per-MCP opt-in is a natural follow-up but would significantly expand scope
• No audit log — the toggle is ephemeral (tracing only); a persistent audit trail could be added separately