feat: make TLS certificate verification configurable

[wavezhang]

Rewrite: per-provider TLS toggle (no global switch)

Rebased and rewritten as a provider-scoped version per review feedback. No global insecure_skip_tls_verify — each [providers.<name>] independently opts in, and it only affects the LLM API client.

Design

• Per-provider only — no global flag; tools, MCP, sandbox, updater, hooks untouched
• Default false — TLS verified unless explicitly opted out
• logging::warn emitted when TLS is disabled for the active provider
• deepseek doctor shows TLS status in both human and --json modes

Usage

[providers.ollama]
base_url = "https://localhost:11434/v1"
insecure_skip_tls_verify = true

Tests
cargo build ✅
cargo test -p codewhale-tui -- insecure_skip_verify build_http_client → 8/8 ✅
cargo test -p codewhale-tui -- status_item → 7/7 ✅

Greptile Summary

This PR adds a per-provider insecure_skip_tls_verify flag to the [providers.<name>] TOML table, letting users accept self-signed or invalid TLS certificates on a provider-by-provider basis without any global bypass switch. The feature is correctly defaulted to false, emits a logging::warn when active, and is surfaced in both the human-readable and --json doctor output.

• ProviderConfigToml and ProviderConfig gain an insecure_skip_tls_verify: Option<bool> field; build_http_client passes the resolved value to reqwest's danger_accept_invalid_certs.
• Config::deepseek_insecure_skip_tls_verify() resolves the flag for the active provider; it is consumed in client.rs and in both run_doctor / run_doctor_json in main.rs.
• merge_provider_config (TUI-side) correctly propagates the new field; a subset of providers in config.example.toml received the commented-out example line.

Confidence Score: 5/5

Safe to merge; the core TLS toggle wires correctly end-to-end and defaults to secure behaviour.

The implementation correctly threads skip_verify through build_http_client, emits a log warning when TLS is disabled, surfaces the setting in both doctor output modes, and merges the field properly in the TUI-side merge_provider_config. The only new issues found in this review pass are naming style and a documentation gap in the example file — nothing that affects runtime correctness.

The merge_project_provider_config function in crates/config/src/lib.rs (already flagged in a prior review thread) still silently drops the new field for project-level configs — worth confirming that is addressed before merge.

Important Files Changed

Filename	Overview
crates/tui/src/config.rs	Adds insecure_skip_tls_verify to ProviderConfig and introduces deepseek_insecure_skip_tls_verify() — a method that applies to any active provider despite its DeepSeek-scoped name. merge_provider_config correctly propagates the field.
crates/config/src/lib.rs	Adds insecure_skip_tls_verify: Option to ProviderConfigToml. However, merge_project_provider_config (line 1527) still only propagates model and path_suffix, so the new field is silently dropped for project-level config files.
crates/tui/src/client.rs	Threads skip_verify through to build_http_client / danger_accept_invalid_certs correctly; emits a logging::warn when disabled; new unit tests cover both false and true cases.
crates/tui/src/main.rs	Adds TLS-disabled warning block to run_doctor output and insecure_skip_tls_verify field to run_doctor_json; both correctly call deepseek_insecure_skip_tls_verify().
config.example.toml	Adds commented-out insecure_skip_tls_verify = false examples for a subset of providers; volcengine, openrouter, xiaomi_mimo, novita, siliconflow, arcee, moonshot, and huggingface sections are missing the line.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[config.toml\nproviders.X.insecure_skip_tls_verify] --> B[ProviderConfigToml]
    B --> C{merge_project_provider_config\ncrates/config}
    C -- "model, path_suffix only\n⚠ insecure_skip_tls_verify dropped" --> D[Merged ProviderConfigToml]
    D --> E[Config::deepseek_insecure_skip_tls_verify\ncrates/tui/src/config.rs]
    E --> F{skip_verify?}
    F -- true --> G[logging::warn emitted]
    F -- true --> H[reqwest::Client::builder\n.danger_accept_invalid_certs true]
    F -- false --> H2[reqwest::Client::builder\n.danger_accept_invalid_certs false]
    E --> I[run_doctor / run_doctor_json\nmain.rs]
    H --> J[DeepSeekClient]
    H2 --> J

Loading

Comments Outside Diff (1)

1. crates/config/src/lib.rs, line 1527-1534 (link)

   merge_project_provider_config silently drops insecure_skip_tls_verify set in a project-level config file. Only model and path_suffix are propagated, so a .codewhale/config.toml entry like [providers.ollama] insecure_skip_tls_verify = true will always be ignored — the effective config retains the user-level value (None/false) even when the project explicitly opts in.

   

_{Reviews (14): Last reviewed commit: "feat: per-provider TLS toggle (rebased)" | 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

[Hmbown]

Thanks for working through the TLS override path. I am keeping this open because GitHub currently reports conflicts, and this is security-sensitive enough that it needs a clean rebase plus full first-party verification before it lands.

The direction is plausible for self-signed/internal endpoints, but please keep the default safe, keep the warnings explicit, and rerun the focused coverage after rebasing: cargo test -p codewhale-config insecure_skip_tls_verify --locked, cargo test -p codewhale-tui insecure_skip_tls_verify --locked, and cargo test -p codewhale-tui skill_install --locked.

[Hmbown]

Thanks @wavezhang. The TLS override direction is useful for internal/self-signed endpoints, but I’m not comfortable merging this branch as-is while it conflicts with current main and changes every outbound HTTPS surface.

One concrete blocker remains in the current head: the TLS banner layout still leaves last_composer_area on chunks[3] instead of the composer chunk. Before this lands, I’d like a clean rebase, that chunk-index fix, first-party focused tests, and ideally a narrower/provider-scoped version rather than a global switch that also affects updater and web/tool traffic.

[Hmbown]

Thanks @wavezhang, this is a useful direction for self-signed/internal endpoints, and I appreciate the follow-up fixes.

I’m going to keep this on hold rather than merge the current branch because it now conflicts with current main and the branch-wide TLS bypass affects every outbound HTTPS surface, including updater, web/tools, hooks, skills, MCP, and sandbox traffic. That is security-sensitive enough that it needs a clean rebase and first-party verification before landing.

One concrete blocker also remains in the current head: the TUI banner layout still leaves last_composer_area/content on the preview chunk instead of the composer chunk. If you continue this PR, please rebase on latest main, adapt the banner to the current header_area/body_chunks layout, and rerun:

cargo test -p codewhale-config insecure_skip_tls_verify --locked
cargo test -p codewhale-tui insecure_skip_tls_verify --locked
cargo test -p codewhale-tui skill_install --locked

We may harvest a narrower provider-scoped version first; if so, I’ll make sure the harvested PR credits your work here.

[Hmbown]

Thanks for the TLS configurability work. This is a real-world need, but it sits in a sensitive part of the trust model, so the UX has to make the tradeoff clear instead of hiding it behind a quiet flag.

I am leaving it unmerged in this pass because the branch is dirty against main. A good next version would rebase cleanly and make the opt-in nature very explicit in config/docs/doctor output, so users understand when certificate verification has been relaxed and why.

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[wavezhang]

Updated — Conflicts resolved + doctor output added

Merge Conflicts Resolved

Rebased onto origin/main with all conflicts addressed. The branch now:

• Keeps proxy AND skip_verify — update.rs has both proxy and skip_verify parameters threaded through run_update, fetch_latest_release, download_url, update_http_client, etc.
• Keeps HEAD's tools field alongside our insecure_skip_tls_verify in EngineConfig and TUI config construction
• Keeps HEAD's hook_sinks config field alongside our insecure_skip_tls_verify config field
• TLS banner preserved in UI layout (banner_height for the warning banner)
• HEAD's allowed_tools field properly initialized

Doctor output now warns when TLS verification is disabled

When insecure_skip_tls_verify = true, the output of codewhale doctor shows:

! TLS certificate verification is DISABLED
    insecure_skip_tls_verify = true — all outbound HTTPS requests will accept
    untrusted certificates. This setting is intended for development and trusted
    internal networks only. In production, keep this disabled (default).

Build status

• cargo check -p codewhale-cli -p codewhale-config — clean (zero warnings)
• codewhale-tui pre-existing errors in origin/main (unrelated to this PR)

What's left

• PR is ready for re-review. The pre-existing codewhale-tui build errors are from origin/main itself and should be resolved separately.

[wavezhang]

@greptile-apps Fixed in commit e4deb86. Both runtime_threads.rs:1971 and main.rs:5428 now call insecure_skip_tls_verify_for_provider(self.config.api_provider()) instead of reading the raw global field, matching the pattern used in build_engine_config in tui/ui.rs.

[greptile-apps]

Confirmed — both paths now use insecure_skip_tls_verify_for_provider(), which ensures env var and per-provider overrides are respected consistently. The fix looks good.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.

[greptile-apps]

MCP subcommands ignore DEEPSEEK_INSECURE_SKIP_TLS_VERIFY env override

All three MCP subcommands (Connect, Tools, Validate) read config.insecure_skip_tls_verify.unwrap_or(false) directly from the raw TOML field. This bypasses the env-var resolution performed by insecure_skip_tls_verify_for_provider(). A user who sets DEEPSEEK_INSECURE_SKIP_TLS_VERIFY=true in the environment without touching the config file will find that mcp connect, mcp tools, and mcp validate all still perform full certificate verification and may fail to reach the same MCP servers that work fine inside a normal session. The same raw-field access recurs on lines 4283 and 4394.

[wavezhang]

@Hmbown Rebased onto latest main (v0.8.49) and rewritten as a provider-scoped version per your feedback:

1. No global switch — insecure_skip_tls_verify lives only in each [providers.<name>] table. Removed from top-level Config.
2. Narrower scope — only affects DeepSeekClient (LLM API). Tools, MCP, sandbox, updater, hooks, skills are all untouched.
3. Doctor output — deepseek doctor shows TLS status in both human and --json modes.
4. Clean rebase — no conflicts against current main (mergeable ✓).
5. 8 tests passing: TOML deserialization, defaults, merge behavior, HTTP client build.
6. Warning — logging::warn emitted when TLS is disabled for the active provider.

Per-provider usage:

[providers.ollama]
base_url = "https://localhost:11434/v1"
insecure_skip_tls_verify = true

cargo build ✅ cargo test -p codewhale-tui -- insecure_skip_verify build_http_client → 8/8 ✅

[greptile-apps]

Want your agent to iterate on Greptile's feedback? Try greploops.

[Hmbown]

Thanks @wavezhang — the latest provider-scoped shape is much closer to the trust boundary I’d want for this. Keeping insecure_skip_tls_verify under [providers.<name>] and away from updater/tools/MCP/sandbox traffic is the right direction, and the doctor output/warning coverage is useful.

For the v0.9.0 stewardship branch I’m still leaving this open rather than harvesting it today because GitHub currently reports the PR as conflicting/dirty against current main, and TLS verification is security-sensitive enough that I want the final branch to be clean and first-party verified before it lands.

A safe next pass would be a fresh rebase against current main or the v0.9 stewardship branch, keeping the scope provider-only, then rerun focused coverage such as:

cargo test -p codewhale-config insecure_skip_tls_verify --locked
cargo test -p codewhale-tui insecure_skip_tls_verify --locked
cargo test -p codewhale-tui build_http_client --locked
cargo check -p codewhale-tui --all-features --locked

Also worth preserving in docs: custom CA via SSL_CERT_FILE should stay the preferred path when users can install/trust a root certificate; this flag is for the self-signed/internal endpoint cases where that is not practical. If we harvest a narrow version later, I’ll keep your work credited.

[Hmbown]

Harvested the narrow provider-scoped TLS configurability slice into #2834, with @wavezhang credited in the PR body, changelog, and commit co-author metadata. Thank you for pushing this forward.

I left this source PR open because the branch is still stale/conflicting and the broader shape is too risky for v0.9.0 as-is. The maintainer PR intentionally keeps the setting disabled by default, scopes it to the active LLM provider client only, keeps SSL_CERT_FILE as the preferred custom-CA path, and does not add any global TLS bypass for updater/tools/MCP/sandbox/web surfaces.

[wavezhang]

✅ Rebased & Tests Passing

This PR has been rebased onto the latest main from Hmbown/CodeWhale (commit 8dff2f75) with all merge conflicts resolved.

What was done

• Rebased onto GitHub main (excludes the static compilation commit from CNB mirror)
• Resolved merge conflicts in 4 files
• Fixed syntax errors from conflict resolution
• Verified all tests pass

Test Results

# Test 1: cargo test -p codewhale-config insecure_skip_tls_verify --locked
→ 87 tests passed ✅

# Test 2: cargo test -p codewhale-tui insecure_skip_tls_verify --locked  
→ 6/6 passed ✅
  - insecure_skip_tls_verify_defaults_to_false
  - insecure_skip_tls_verify_toml_deserializes_true
  - insecure_skip_tls_verify_toml_explicit_false
  - insecure_skip_tls_verify_toml_missing_defaults_false
  - merge_provider_config_insecure_skip_tls_verify_falls_back
  - merge_provider_config_insecure_skip_tls_verify_override_wins

# Test 3: cargo test -p codewhale-tui build_http_client --locked
→ 2/2 passed ✅
  - build_http_client_with_skip_verify_false
  - build_http_client_with_skip_verify_true

# Test 4: cargo check -p codewhale-tui --all-features --locked
→ Compilation successful ✅

Clean Commit History

• ✅ Single commit: feat: per-provider TLS toggle (rebased)
• ✅ Author: wavezhang <wavezhang@users.noreply.github.com>
• ✅ No static compilation commits included
• ✅ Scope strictly provider-only

Ready for review! 🚀