feat(webui): deep-scan master control on the Security page, with truth badges - #952
Merged
Merged
Conversation
…idence Owner report against v0.53.0-rc.7: the TPA scanner raised a dangerous finding because ElevenLabs and kaggle both expose list_models. A name collision alone is the proxy's normal operating condition — mcpproxy unifies many servers, every tool is namespaced server:tool, and retrieve_tools' BM25 ranking is what disambiguates shared compound names. No fixed "distinctive name" heuristic separates those from attacks. - Collision branch now requires impersonation-clone evidence: the same name on another server AND a near-duplicate description (token-set containment; cosmetic case/punctuation/whitespace edits do not launder a copy). Bare coincidences are silent from both sides. - Reference branch no longer fires when the tool's OWN server also exposes the referenced name — "call list_models first" is ordinary self-documentation, whoever else exposes that name. - Corpus: the two shadowing entries now model the real clone attack (they were bare collisions with honestly different descriptions — the exact FP class); new hard-negative hn_name_coincidence encodes the ElevenLabs/kaggle case as benign. Unicode escapes preserved. - Docs updated (tool-scanner.md, security-quarantine.md).
… robustness - cmd/scan-eval gateFixture and the sh_collide_transfer corpus entry modelled bare collisions (or byte-identical dups); both now model the cosmetic-variant clone the check actually detects, so the gate measures the real attack shape. - hn_name_coincidence renamed hn_shadowing_name_coincidence per the corpus id-prefix convention (detect_corpus_test hardNegPrefix). - descTokens is unicode-aware: a Cyrillic/CJK description tokenizes to real words instead of an empty set that could never evidence a clone. - cloneDescriptions requires >=3 tokens per side: "Create" == "Create" carries no information, and sub-minimal identical descriptions no longer hard-flag a shared name. - The two accepted trade-offs codex argued (different-description collisions out of scope; own-server decoy silencing the reference branch) are now documented at the decision sites: both reduce to the name-coincidence policy the owner set (MCP-3520), whose defenses are admission quarantine, server:tool namespacing, and retrieve_tools provenance — not a collision alarm.
…codex round 2) FieldsFunc finds no word boundaries in unspaced Han/kana/Hangul/Thai text, so an informative CJK description collapsed to ONE token and ducked under the three-token evidence floor — a cloned unspaced description could never be detected, and the round-1 test had cheated with artificial spaces. Runs in spaceless scripts now emit character bigrams (the standard segmentation-free indexing unit), which restores both the floor and the containment thresholds for those scripts. The boundary itself is now pinned: two identical tokens carry no clone evidence, three do.
…h badges Owner report: five scanners toggled "enabled" in the Web UI, every scan report tpa-only — the deep-scan master gate (Spec 077) was invisible on the one page where its effect shows. The page pointed at Settings in a paragraph (spec 088 FR-018); a paragraph is not a control. - A "Deep scan" card now heads the scanners page: live on/off state, a toggle wired to the same config field Settings writes (PATCH /api/v1/config, hot-reloaded), a one-line statement of what a scan will actually run right now, and a Docker warning when the layer is on but Docker is not. - The truth badge: an enabled Docker scanner under a disabled deep-scan layer shows an amber "won't run" instead of a green "enabled" — the row-level lie is the thing this page existed to prevent. The built-in baseline (no docker_image) is exempt: it always runs, and calling it "won't run" would be the same lie in the other direction. `null` config state never accuses (no flicker on load). - Logic lives in views/security/deepScanState.ts, pure and unit-tested (13 cases incl. the baseline exemption and the not-yet-loaded state). Verified end-to-end in Chrome against an isolated instance: toggle flips the badge and persists to the config file; Settings reflects the shared state; per-server trust_mode (auto/scan/manual, spec 086/088) confirmed already configurable on the server Configuration tab.
Deploying mcpproxy-docs with
|
| Latest commit: |
323f53e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0fd3714c.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://feat-webui-security-config.mcpproxy-docs.pages.dev |
…esh, copy
- handlePatchConfig now serializes its read-merge-apply under a mutex:
two concurrent config PATCHes (Security page + Settings in another
tab) both merged from the same snapshot and the later full-config
apply silently dropped the earlier change. Pre-existing hazard for
every PATCH caller, closed for all of them.
- A rejected PATCH now snaps the deep-scan checkbox back explicitly:
the browser flips the DOM before the request fails, and Vue sees an
unchanged :checked prop, so toggle/badge/summary could disagree.
- The card refreshes on window focus, so a Settings change in another
tab cannot leave the truth badges stale beyond a glance away.
- Copy: pass-1 Docker scanners run alongside the baseline, not after
it ("after" described pass 2).
…nd 2) An unmount during the startup awaits ran onUnmounted first and the focus/scanner listeners were added afterward — leaked with nothing left to remove them. Listeners now register synchronously at mount.
Member
Author
|
Codex critic — three rounds on this PR, final verdict CLEAN:
Also screenshot-verified end-to-end in Chrome against an isolated instance: enable scanner → amber "won't run"; card toggle → badge flips, config file persists, Settings shows shared state; per-server trust_mode confirmed already live on the server Configuration tab. |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 30886647667 --repo smart-mcp-proxy/mcpproxy-go
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Owner report: five scanners toggled "enabled" in the Web UI, every scan report tpa-only. The deep-scan master gate (Spec 077,
security.deep_scan.enabled) was invisible on the one page where its effect shows — the scanners page said "enabled" in green for scanners that would never run, and pointed at Settings in a paragraph (spec 088 FR-018). A paragraph is not a control.Change (frontend-only)
PATCH /api/v1/config, hot-reloaded by the core — the two controls cannot disagree for more than one refresh), a one-line statement of what a scan will actually run right now ("Off — scans run only the built-in offline baseline. The N scanners enabled below will not run until deep scan is on."), and an inline Docker warning when the layer is on without Docker.tpa-descriptions, nodocker_image) is exempt — it always runs, and calling it "won't run" would be the same lie in the other direction. Unloaded config (null) never accuses, so no badge flicker on load.views/security/deepScanState.ts, pure and unit-tested (13 cases including the baseline exemption and the not-yet-loaded state). No backend changes; no new dependencies.Verified live (Chrome, isolated instance)
Enable a scanner with deep scan off → amber "won't run"; flip the card toggle → toast, badge turns green "enabled",
deep_scan.enabled: truelands in the config file, Settings shows the shared state. Per-servertrust_mode(auto/scan/manual — specs 086/088) confirmed already configurable on the server Configuration tab, so all three security controls from the operator questions now have UI.Frontend: 588 unit tests green, vue-tsc clean.