Skip to content

feat: surface the mitm configuration chapter in the editors - #49

Open
TiaraBasori wants to merge 2 commits into
ProjectClash:mainfrom
TiaraBasori:feat/mitm
Open

TiaraBasori wants to merge 2 commits into
ProjectClash:mainfrom
TiaraBasori:feat/mitm

Conversation

@TiaraBasori

Copy link
Copy Markdown

Summary

  • Registers the new top-level mitm: block across the configuration editing surface: outline section with On/Off state and rewrite-policy counts, trilingual field copy, a chapters layout, list-validation paths, and the governing-switch audit.

Motivation

Companion to ProjectClash/Clash#8, which adds the kernel-side component/mitm engine behind the same mitm: block. Without this PR the editors treat mitm as an unknown key and the configuration center cannot show or edit it.

Changes

  • ClashClientKit — ConfigurationOutlineSection.mitm (ordered after sniffer), outline row counting mitm.rewrite with the enable state, mitm.skip-domain in the trie-domain validation paths, mitm.skip-dst-address in the IP-matcher paths, mitm.enable in the audited governing switches.
  • ClashClientUI — FieldCopy/a-mitm.json: titles/summaries for every mitm.* path (en, 简体, 繁體), including the rewrite.* object-list and headers.* sub-entries; layouts-chapters.json: a MITM chapter with Basics / Scope / Rewrite Policies; mitm.enable and the six mitm list paths pinned in the upstream-default tables (preconditionFailure guards require the pins).
  • ClashClient / ClashMacClient — every exhaustive switch over ConfigurationOutlineSection extended for the new case (route, copy, value, inspector destinations, search-index page set, On/Off rendering).

Field-level definitions are intentionally not duplicated here: the editors' field catalog is reflected from RawConfig by the kernel's SyntaxCatalogForIOS, so mitm.* fields appear automatically once the kernel PR's SDK ships.

Test Plan

  • Static exhaustive-switch audit over every file referencing ConfigurationOutlineSection: no switch on the enum lacks .mitm or a default arm.
  • Both FieldCopy JSON files parse; chapters layout includes the mitm chapter with the pinned keys.
  • Upstream-default pins cover every mitm list path the editors can touch (no preconditionFailure on first open).
  • Xcode build on macOS with Xcode 27 (this machine is Linux; CI or a local checkout should run the schemes before merge).

Notes for Reviewers

  • The CA install-and-trust UI (reading MITMState/ExportMITMCa from the core) needs the new exports in a released SDK; it is deliberately left out here so this PR compiles against the current Dependencies.lock.json. It lands as a follow-up once the kernel PR merges and a new SDK is pinned.

Registers the new top-level mitm: block (companion to the kernel's
component/mitm) across the configuration surface: outline section with
On/Off and policy counts, field-copy strings (en/zh-Hans/zh-Hant), a
chapters layout (Basics / Scope / Rewrite Policies), domain and CIDR
list validation paths, the governing-switch audit, upstream default
pins, and the Mac/iOS/tvOS switch coverage for the new section.

Field-level definitions ride the kernel's syntax catalog (reflected
from RawConfig), so no local field table is needed. The CA install
flow needs the MITMState/ExportMITMCa exports from a released SDK and
lands as a follow-up once the kernel PR ships.
Review found the eight rewrite-child copy keys using the '.*' wildcard
form while the kernel's reflected catalog emits objectList children as
'mitm.rewrite[].url-pattern' etc., leaving all eight records unreachable
(editors showed raw YAML key names). Rekeyed to the '[]' form.

Also: the iOS outline's MITM row now shows On/Off like the Mac card and
the sniffer row (mitmEnabled mirrors snifferEnabled), the url-pattern
summary states the actual suffix-match semantics (host and subdomains),
value-prefix copy reflects the new prepend-to-client-value behavior,
and the mitm chapter gains the standard 'More Settings' catchall so
future kernel keys surface under a curated header.
@TiaraBasori

Copy link
Copy Markdown
Author

Pushed 7ef415e from the review pass:

  • The eight rewrite-child copy keys used the .* wildcard form, but the kernel's reflected catalog emits objectList children as mitm.rewrite[].url-pattern etc. — all eight records were unreachable (editors showed raw YAML key names). Rekeyed to the [] form, verified against a-general.json's tunnels[].network precedent and a scripted cross-check of every key against the kernel's RawMITM/RawMITMRewrite/RawMITMHeader yaml tags.
  • iOS outline now shows On/Off for the MITM row (parity with the Mac card and the sniffer row).
  • url-pattern summary corrected to the kernel's actual suffix semantics (host and subdomains); value-prefix copy updated for the new prepend-to-client-value behavior (kernel-side, Clash#8).
  • Standard "More Settings" catchall appended to the mitm chapter (matches dns/tun/sniffer).

@TiaraBasori TiaraBasori left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hermes Agent Review (round 2 — paired with core PR #8)

Cross-checked against the kernel PR (ProjectClash/Clash#8, same feature). Client-side surface looks solid overall: chapter wiring is complete and consistent across iPhone/Mac editors (outline section, route, governing-switch audit list, trie/IP validation paths, pinned defaults, On/Off parity in both editors), and the zh-Hans/zh-Hant/en copy is accurate against the kernel's actual semantics (verified: +.example.com suffix semantics, first-match policies, bare-host redirect keeping path+query, value-prefix prepending, reject 4xx, default ports 443/80).

Findings (all minor — nothing blocking found):

  1. ⚠️ No mention of the CA install step as a dependency. The copy says "启用前请先在应用内安装并信任 CA" but nothing in the editor links to the CA surface (MITMState/ExportMITMCa added in the core PR's bind layer). A user can flip mitm.enable and every gated site breaks with TLS errors until they find the CA export elsewhere. Consider a footer link or warning state when enable=true and no profile is installed (the core exposes MITMState for exactly this).

  2. ⚠️ store-dir free-text path is a footgun on iOS. The kernel persists the CA under whatever path the client passes; the editor exposes it as a raw string field. On iOS a user-entered path either points outside the app-group container (write fails → MITM silently disabled, executor logs initial mitm failed) or inside it (fine but obscure). Better to hide it behind an expert toggle or manage it app-side rather than exposing it as an editable field.

  3. 💡 Outline count for .mitm counts rewrite entries (ConfigurationOutline.swift: policies = field(mitm, "rewrite")) — consistent with sniffer's sniff count, fine; but hosts being empty with enable: true is the misconfiguration the kernel warns about at load ("hosts are configured but no rewrite policies" is the other one). Neither is surfaced client-side; a small validation hint would help.

  4. 💡 layouts-chapters.json "More Settings" catch-all (*) now picks up nothing unexpected — checked against the kernel's RawMITM field set (enable/store-dir/hosts/skip-domain/skip-dst-address/https-ports/http-ports/rewrite); all six listed keys in the chapters match real fields, no typos.

  5. 💡 mitmEnabled(in:) duplicates the sniffer on/off extraction logic; a shared helper would avoid drift, but consistent with existing style.

Verified locally that the three Swift packages touched compile-clean is not possible in this environment (no Xcode toolchain); the diff is confined to additive enum cases + JSON resources, and every switch over ConfigurationOutlineSection in the diff was extended (checked: EditorViews, OnePage route, Mac inspector, Mac outline card, Mac page objects, ConfigurationOutline sections/defaults — no non-exhaustive switch left behind).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant