feat(security): US3 opt-in deep scan (off by default) + config migration (Spec 077)#793
Merged
Conversation
…tion
Add the opt-in security.deep_scan config group (Spec 077 US3) that subsumes the
deprecated top-level scanner_fetch_package_source / scanner_disable_no_new_privileges
keys and gates the heavy Docker scanner layer. deep_scan.enabled defaults false so
only the deterministic in-process baseline runs.
- Add DeepScanConfig{Enabled, FetchPackageSource, DisableNoNewPrivileges, Scanners}
with swaggertype tags; add nil-safe effective accessors (IsDeepScanEnabled,
DeepScanScanners, EffectiveFetchPackageSource, IsDisableNoNewPrivileges).
- Remove the orphaned auto_scan_quarantined key (ignored on load if present).
- migrateDeepScanConfig folds the deprecated top-level keys into deep_scan.* on
every load/hot-reload (wired in initializeRegistries) and clears the legacy
keys so a re-serialized config exposes only the new surface. Idempotent.
- Round-trip + ignore-removed-key tests.
Related: Spec 077 (specs/077-scanner-simplification)
…seline
Demote the Docker scanner plugins + source extraction to an opt-in "deep scan"
layer (Spec 077 US3). Off by default: only the deterministic in-process baseline
runs and no Docker is invoked; a deep-scan failure is surfaced informationally
and never changes the baseline verdict.
- engine: gate Docker (non-in-process) scanners on deep_scan.enabled via
deepScanAllowed; when off, resolveScanners drops them entirely so no container
runs and no failure can degrade the verdict. Optional per-scanner allow-list.
- service: SetDeepScan runtime knob; populate DeepScanDescriptor
{enabled,ran,available,scanners_failed} from per-scanner job statuses,
classifying baseline (in-process) vs deep scanners.
- service: remove degradeIfIncompleteCoverage — the scan verdict is now derived
solely from baseline findings (FR-008/FR-014); a failed deep scanner no longer
downgrades a clean baseline to "degraded".
- server: wire security.deep_scan.* into the scanner service; read the
no-new-privileges / fetch-package-source knobs via the effective accessors so
migrated configs resolve to the unified surface.
- Point user-facing hints/logs at the new deep_scan.* config keys.
- Tests: deep-scan-off-by-default (baseline only, no Docker) and deep-scan
failure leaves the baseline verdict unchanged with a populated descriptor;
update the MCP-34.4 Docker-scanner tests to enable deep scan and the former
MCP-2401 degrade tests to assert the baseline-only verdict.
Related: Spec 077 (specs/077-scanner-simplification)
Regenerate oas/swagger.yaml + oas/docs.go for the new config.DeepScanConfig schema and the removed auto_scan_quarantined key. make swagger-verify passes. Related: Spec 077 (specs/077-scanner-simplification)
Spec 077 US3 Web UI: present deep scan as an opt-in affordance and render a failed/unavailable deep scanner as informational, never an error — the baseline verdict is authoritative. - Security.vue: info banner clarifying the deterministic baseline is always on and the Docker scanners are an opt-in deep scan that never blocks/degrades it. - ScanReport.vue: DeepScanDescriptor info block (alert-info) listing unavailable deep scanners with an explicit "does not affect the baseline verdict" note. - api.ts: DeepScanDescriptor type + optional deep_scan on the report. Related: Spec 077 (specs/077-scanner-simplification)
…eport banner Address adversarial review findings on Spec 077 US3 (PR #789): - #1 (MUST): published-package-source extraction is now part of the opt-in deep-scan layer. server.go computes the effective fetch as IsDeepScanEnabled() && fetch_package_source (default true), and SetDeepScan(false) force-disables the resolver's fetch fallback as defense-in-depth. Deep scan OFF (default) => no npx/uvx source-fetch network egress. Added TestServiceDeepScanGatesPackageSourceFetch and updated the doc comments to match. - #2 (MUST): the deep-scan report banner was dead code — AggregatedReport had no DeepScan field, so /scan/report and /scans/{jobId}/report never emitted deep_scan. Added DeepScan *DeepScanDescriptor (json deep_scan, omitempty) to AggregatedReport and populate it in GetScanReport and GetScanReportByJobID (the live report-page path). - #3 (LOW): buildDeepScanDescriptor now inspects Pass-1 AND Pass-2 scanner statuses (variadic jobs, deduped) so heavy trivy/supply-chain scanner failures are reflected in scanners_failed/availability. Baseline verdict logic and the quarantine state machine are unchanged. Related: Spec 077
Deploying mcpproxy-docs with
|
| Latest commit: |
a28b617
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2e86532f.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://077-us3-deep-scan-optin.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 28605323264 --repo smart-mcp-proxy/mcpproxy-go
|
🤖 Codex cross-model review — PR #793 US3 (round 1, read-only)Post-rebase review. Confirms the rebase-critical items are intact (tier-driven gate, degrade-removal, single DeepScanDescriptor). Finds a real gap the earlier fix missed: deep-scan-OFF still runs source resolution + Pass 2 (Docker/extraction/egress), and the REST/SSE contract lacks deep_scan. Round 1 of the 5-round cap. Posted via the gatekeeper app. VERDICT: changes_requested Findings
Verified safe
|
…scan layer Spec 077 US3 promised that with deep scan OFF (the default) only the deterministic in-process baseline runs — no Docker, no source extraction, no network egress. Scanner EXECUTION was already gated, but source RESOLUTION passes still ran. Close the three gaps: - Finding #1 (HIGH): gate Pass-1 sourceResolver.Resolve on deepScanEnabled(). With deep scan off, source resolution is skipped entirely (the baseline scans tool DEFINITIONS, not source files, so resolved source is unused) — no Docker container lookup/extraction and no package-source fetch. - Finding #2 (HIGH): gate the Pass-2 auto-start (startPass2 → ResolveFullSource, the heavy Docker/full-source pass) on deepScanEnabled() in addition to the existing not-dry-run/not-URL conditions. Off ⇒ Pass-1 baseline only. - Finding #3 (MEDIUM): add deep_scan (DeepScanDescriptor: enabled/ran/ available/scanners_failed) to contracts.SecurityScanSummary, populate it in the server.go enricher adapter (was dropped), remove stale "degraded" references from the contract comments, and regenerate OpenAPI. Tests: SourceResolver gains atomic Resolve/ResolveFullSource call counters; new service-level tests assert neither runs with deep scan off (baseline still settles a deterministic verdict) and both run with deep scan on; new server adapter test asserts deep_scan is carried onto the wire summary when populated and omitted when off. Invariants preserved: isBlockingFinding stays tier-driven; no degradeIfIncompleteCoverage; determinism/offline baseline unchanged. Related: Spec 077
Dumbris
force-pushed
the
077-us3-deep-scan-optin
branch
from
dd48e0e to
a25ae2f
Compare
🤖 Codex cross-model review — PR #793 US3 (round 2, read-only)Round-1 egress/contract fixes verified (deep-off now skips Docker/Pass2/source-resolution; deep_scan on the wire contract). Round 2: two real config-wiring bugs — deep_scan changes don't hot-reload (DetectConfigChanges ignores Config.Security) and /api/v1/config/apply skips the legacy-key migration. Round 2 of the 5-round cap. Posted via the gatekeeper app. VERDICT: changes_requested Findings
Verified safe
Verification commands:
|
…pply (Spec 077 US3) Codex round-2 findings on PR #793: Finding #1 (HIGH) — deep_scan changes now hot-reload. DetectConfigChanges compares Config.Security (deep_scan.{enabled,fetch_package_source, disable_no_new_privileges,scanners} + deprecated top-level keys) so a lone security.deep_scan.* edit is reported as a change instead of "no changes detected". The server subscribes to config.reloaded (both file-edit and /api/v1/config/apply emit it) and re-runs the scanner wiring via a new Service.ApplySecurityConfig, so toggling deep scan takes effect without a restart. Startup and reload now configure the scanner through the same call. Finding #2 (MEDIUM) — /api/v1/config/apply now normalizes identically to LoadFromFile. ApplyConfig runs config.MigrateDeepScanConfig on the submitted config before diffing/saving, folding the deprecated security.scanner_fetch_package_source / scanner_disable_no_new_privileges keys into security.deep_scan (auto_scan_quarantined has no struct field, dropped at decode). An API apply carrying the deprecated keys now saves only the unified deep_scan surface (SC-007). Tests: DetectConfigChanges deep-scan detection; Service.ApplySecurityConfig reconfigures a live service without restart (incl. legacy-key fallback); ApplyConfig legacy-key migration asserted on the saved file + runtime config. Constraints respected: no new dependency; tool_quarantine.go untouched; no US1-removed behavior reintroduced; determinism/offline preserved. Related: Spec 077
Dumbris
added a commit
that referenced
this pull request
Jul 2, 2026
…iorities, audit fixes (#797) A multi-agent consistency audit (2026-07-02) found roadmap.yaml stale versus merged PRs and carrying several false progress badges from wrong spec links. Corrected CI-filtered telemetry also re-prioritized the personal-edition work. Statuses corrected per merged PRs: - scanner-simplification children: US1 #786 / US2 #792 / US4 #794 marked done; US3 #793 in_review. Epic stays in_progress. Added deep-scan trust-fix task and flagged docs T037-T039 as merge-blocking for #793. - registries-official-protocol marked done (spec 071 shipped 12/12, #572). False badges / wrong provenance fixed (per the file's own convention — link dropped, provenance moved into the note): - sandbox-isolation no longer links spec 054 (unrelated security-gateway spec). - ux-audit no longer links spec 064 (unrelated agent-fleet cockpit spec). - marketplace no longer links spec 070 (that is the registries-search-add spec). - action-log-transparency no longer links spec 024 (shipped backend, not the progress driver for the at-a-glance UX epic). New epics (telemetry- and audit-driven replan): - upgrade-nudge (P0, spec 079): ~60% of active installs run pre-v0.40. - connect-trust (P0, spec 078): 72.4% skip the connect step. - telemetry-identity (P1, in_progress): hashed machine_id + CI-filter hardening. - planning-hygiene (P2): automate the checks this audit did by hand. Windows QA gate: new first child windows-tray-funnel-qa (downloads→actives 12:1 vs macOS 4:1); windows-tray-window now depends on it. ROADMAP.md regenerated; gen-roadmap.py --check passes. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…always-on deep_scan descriptor, enable-time hint Three verified audit findings against the Spec 077 US3 deep-scan layer (re-checked against head 7e1d51a before fixing): FIX 1 (nil-Security gating) — already fixed at HEAD by a25ae2f/7e1d51ad: source resolution + Pass 2 are gated on deepScanEnabled() at the call sites and ApplySecurityConfig(nil) forces the layer off via nil-safe accessors. This commit adds defense-in-depth (the server wiring now calls ApplySecurityConfig unconditionally, even when Config itself is nil) and a regression test pinning the exact audit scenario: config.DefaultConfig() never initializes Config.Security, and that nil block must still yield deep-scan OFF, no Docker scanners resolved, package-source fetch OFF. FIX 2 (FR-014 verdict purity) — the "warnings" level was driven by ThreatLevel across ALL findings, so a tierless deep-scan/external finding at threat_level=warning flipped a clean baseline to "warnings", while a tierless threat_level=dangerous finding fell into the Info bucket (an inversion: LESS effect than warning). GetScanSummary now derives the verdict at EVERY level solely from baseline (tiered) findings: dangerous requires >=1 hard-tier finding (unchanged predicate, shared with the approval gate), warnings requires >=1 warning-level baseline (soft) finding. Tierless findings still surface in FindingCounts — a tierless dangerous now counts at warning prominence instead of info — and in the merged report/RiskScore (FR-009..FR-012 consensus weighting untouched), but they never move the verdict. FIX 3 (silent Docker-scanner skip): (a) buildDeepScanDescriptor no longer returns nil when the layer is off; it always emits {enabled:false, ran:false, skipped_scanners:[ids of enabled-but-skipped Docker scanners]}, making quickstart scenario 1 (deep_scan.enabled=false) actually observable in scan/report JSON. Field added to scanner + contracts descriptors, REST/SSE projection, frontend TS type; OpenAPI regenerated. (b) POST /security/scanners/{id}/enable now returns a "hint" when a Docker-based scanner is enabled while security.deep_scan.enabled is false, and `mcpproxy security enable <id>` prints it ("scanner enabled, but it will not run until security.deep_scan.enabled=true"), plus help-text. SecurityController grew DeepScanEnabled() (implemented by scanner.Service already). Assumptions documented (zero-interruption policy): - Tierless threat_level=dangerous findings are bucketed as Warning in FindingCounts (inform-without-gating prominence) so counts cannot contradict the tier-driven Dangerous count / verdict. - SkippedScanners lists installed/configured (i.e. enabled) non-in-process scanners only; merely "available" scanners are not "enabled" and are not listed. - TestGetScanSummaryBothPasses and the descriptor-omitted assertions encoded the pre-FR-014 behavior and were updated to the spec-mandated contract. Tests: new regression tests for all three fixes (scanner service, httpapi handler, CLI hint extraction); go test -race across security/server/ config/httpapi/contracts/cmd green; golangci-lint v2 clean; frontend vue-tsc clean; scripts/test-api-e2e.sh 65/65. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ier-driven verdict with the report payload Codex review follow-up. The server-list summary (GetScanSummary) derived the verdict tier-driven/baseline-only (FR-014), but the report page badge still read the RAW threat-level ReportSummary — with deep scan on, a tierless Docker-scanner finding classified "dangerous" made the report say dangerous while the server list said clean, and the same finding counted as Warning on one surface and Dangerous on the other. - extract deriveBaselineVerdict() as the single source of truth, shared by GetScanSummary and AggregateReports so the two surfaces structurally cannot disagree - AggregatedReport gains Verdict + tier-driven FindingCounts (additive; raw Summary counts retained for transparency) - ScanReport.vue: status badge, threat tiles, Quarantine-button gate and the Approve-disable gate (hasUnresolvedCritical) now read the tier-driven verdict/finding_counts (raw summary only as a fallback for pre-Spec-077 payloads); the approve gate now mirrors the backend hard-tier-only isBlockingFinding instead of raw summary.critical - ServerDetail.vue: approve-confirmation count and the Security-tab summary strip prefer tier-driven finding_counts over the raw report summary, so a tierless deep-scan "dangerous" finding no longer triggers the "Dangerous Findings Detected" modal on a clean baseline - frontend/src/types/api.ts: SecurityScanSummary catches up with the wire contract (deep_scan, scanners_run/failed/total); SecurityScanReport gains verdict + finding_counts Tests: engine-level AggregateReports verdict matrix (tierless-dangerous → clean/warning-bucket, hard → dangerous, soft → warnings) and an end-to-end pin that GetScanReportByJobID.Verdict/FindingCounts equal GetScanSummary for the same scan data. Assumption documented (zero-interruption policy): ServerCard.vue was reported as sharing the raw-summary preference but already reads only the tier-driven security_scan.finding_counts — left unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
Deep-scan audit follow-up (2 commits:
|
Dumbris
added a commit
that referenced
this pull request
Jul 2, 2026
…te path
First slice of specs/078-connect-trust-preview US1 (see spec FR-001..004,
012, 013). Adds GET /api/v1/connect/{client}/preview returning the exact
change a subsequent connect would make — target config_path, server_key,
entry name, and the exact entry contents — WITHOUT modifying the file or
creating a backup.
Preview==write guarantee: buildServerEntry is now the single construction
path for every client, including Codex/TOML (connectTOML previously built
its {url} entry inline). Preview derives its entry from the same function
the write uses, so what is previewed equals what is written (FR-002); a
table-driven test connects each supported client and asserts the written
file entry deep-equals the shared constructor output across JSON and TOML.
API-key honesty (FR-004): the embedded apikey is masked in the payload
(entry, entry_text) while contains_api_key flags that a credential is
written; the real key never appears in the preview (verified by test that
marshals the whole payload and asserts the secret is absent).
Spec 075 access states (FR-012): the on-demand read used to classify
create-vs-overwrite degrades to access_state=absent|malformed and a denial
returns the same typed AccessError (403 + remediation) as connect/disconnect.
- internal/connect/preview.go: Service.Preview, maskURLAPIKey, entry render
- internal/connect/clients.go: buildServerEntry gains the codex case
- internal/connect/connect.go: connectTOML uses buildServerEntry
- internal/httpapi: handler + route + tests (masked, no side effects, 403)
- oas: regenerated for the new endpoint (make swagger)
Tests: preview-equals-write (JSON+TOML), masking, entry_exists (create vs
overwrite), no-side-effects (no write, no backup, mtime unchanged), absent/
malformed/denied degradation. go test -race ./internal/connect/... and
./internal/httpapi/... pass; golangci-lint v2 clean.
Related #793
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dumbris
added a commit
that referenced
this pull request
Jul 2, 2026
…t modal
Second slice of specs/078-connect-trust-preview US1 (FR-001,003,004): the
Connect action in BOTH the standalone Connect modal and the onboarding
wizard's ClientRow now fetches the preview first and shows an inline panel
before any file is written:
- target config path and, in a monospace additive ("+ will be added")
block, the exact entry that will be written (pretty JSON, or TOML for
Codex) with the API key masked;
- plain-language trust copy ("Only this entry is added — everything else in
the file stays untouched. A timestamped backup is created first.");
- a masked-key line when contains_api_key so the user knows a credential is
written;
- an overwrite warning when an entry with the same name already exists, in
which case confirming implies force=true;
- graceful copy for the bridge/no-prior-file (access_state=absent) and
unparseable (malformed) cases.
Confirm proceeds with the connect (reusing the existing #799 post-connect
backup-path line, untouched); Cancel dismisses the panel and writes nothing.
A non-denial preview fetch error is surfaced inline; in the modal a denied
read still resolves the Spec 075 access banner via checkAccess. Wizard
connect-step telemetry (markConnectCompleted/Skipped) is unchanged.
- types/api.ts: ConnectPreview; services/api.ts: getConnectPreview
- ConnectModal.vue + OnboardingWizard.vue ClientRow: preview panel + flow
- The existing #799 connect specs are updated to drive click → preview →
confirm (Connect no longer writes on the first click); new
connect-preview.spec.ts covers preview render, cancel-writes-nothing,
confirm-proceeds, masked key, and overwrite→force for both surfaces.
Full frontend suite: 33 files, 244 tests pass; vue-tsc clean; make build
embeds the updated dist. npm-ci lockfile churn reverted (as in #799).
Related #793
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dumbris
added a commit
that referenced
this pull request
Jul 2, 2026
…e carrier
Security fix folded into Spec 078. Connect (and the new preview) previously
appended ?apikey=<REST-admin key> to EVERY client entry via mcpURL(), but
/mcp does not require auth by default (config.RequireMCPAuth defaults false;
the admin context is granted unauthenticated). So connect leaked the
REST-admin API key in plaintext into other apps' config files where it
served no purpose.
New behavior, threaded from config.RequireMCPAuth into connect.Service the
same way listenAddr/apiKey are (server wiring + both CLI sites), via
WithRequireMCPAuth:
- require_mcp_auth=false (default): the entry carries NO credential — a clean
http://host:port/mcp URL, no query, no headers. Existing configs keep
working because /mcp accepts unauthenticated requests when protection is off.
- require_mcp_auth=true: the credential is written via the carrier each
client's real config schema supports (verified against vendor docs):
* X-API-Key header — claude-code, vscode, cursor, windsurf, gemini,
opencode (all support a "headers" object on their remote entry);
* mcp-remote --header "X-API-Key:<key>" bridge arg — claude-desktop
(no space after the colon: Claude Desktop/Cursor don't escape spaces in
args and mangle the header — geelen/mcp-remote README; our keys are
space-free);
* ?apikey= query fallback — codex only, whose TOML HTTP transport takes
header values indirectly via env-var names (http_headers /
bearer_token_env_var) and cannot express a literal header value.
Server-side ExtractToken accepts X-API-Key, Authorization: Bearer, and
?apikey= (in that order), so header and query authenticate identically.
buildServerEntry now takes serverEntryParams{baseURL, credential}; mcpURL()
is replaced by the credential-free baseURL() anchor. Entry MATCHING
(findEntry*/findEquivalentJSONServerName/entryPointsToBridge) already keys on
baseURL with an optional ?query, so it recognizes BOTH legacy ?apikey=
entries and new clean entries — reconnect adopts/updates in place (no
duplicate) and disconnect still finds legacy entries; a force-reconnect over a
legacy entry upgrades it and drops the leaked key.
Preview reflects reality: contains_api_key is true only when require_mcp_auth
is on; the credential is masked in whichever carrier it uses (header value,
bridge arg, or query) and the real key never appears in the preview payload.
Tests: matrix over require_mcp_auth on/off × all 8 clients for both write and
preview (preview==write shape, auth-off writes contain NO apikey/headers at
all); per-client carrier assertions; legacy ?apikey= migration (recognized,
upgraded in place, no duplicate, disconnect still works). go test -race
./internal/connect/... ./internal/httpapi/... pass; server/config/management
pass; golangci-lint v2 clean.
Related #793
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dumbris
added a commit
that referenced
this pull request
Jul 2, 2026
Follows the connect security fix: the preview panel's masked-credential line now reads "This entry includes your API key (shown masked)…" instead of "The URL includes…", since with require_mcp_auth on the credential is carried in an X-API-Key header (or the mcp-remote --header bridge arg) for most clients and only falls back to the ?apikey= URL query for Codex. The line already renders only when contains_api_key is true, which the backend now sets only when require_mcp_auth is on — so the default (keyless) connect shows no credential line at all. Adds a frontend test for the default keyless case: contains_api_key=false ⇒ no credential line, and the previewed entry shows neither an apikey query nor a mask. Full frontend suite: 33 files, 245 tests pass; vue-tsc clean. Related #793 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dumbris
added a commit
that referenced
this pull request
Jul 2, 2026
… the API key into client configs (Spec 078 US1) (#802) * feat(connect): preview endpoint sharing buildServerEntry with the write path First slice of specs/078-connect-trust-preview US1 (see spec FR-001..004, 012, 013). Adds GET /api/v1/connect/{client}/preview returning the exact change a subsequent connect would make — target config_path, server_key, entry name, and the exact entry contents — WITHOUT modifying the file or creating a backup. Preview==write guarantee: buildServerEntry is now the single construction path for every client, including Codex/TOML (connectTOML previously built its {url} entry inline). Preview derives its entry from the same function the write uses, so what is previewed equals what is written (FR-002); a table-driven test connects each supported client and asserts the written file entry deep-equals the shared constructor output across JSON and TOML. API-key honesty (FR-004): the embedded apikey is masked in the payload (entry, entry_text) while contains_api_key flags that a credential is written; the real key never appears in the preview (verified by test that marshals the whole payload and asserts the secret is absent). Spec 075 access states (FR-012): the on-demand read used to classify create-vs-overwrite degrades to access_state=absent|malformed and a denial returns the same typed AccessError (403 + remediation) as connect/disconnect. - internal/connect/preview.go: Service.Preview, maskURLAPIKey, entry render - internal/connect/clients.go: buildServerEntry gains the codex case - internal/connect/connect.go: connectTOML uses buildServerEntry - internal/httpapi: handler + route + tests (masked, no side effects, 403) - oas: regenerated for the new endpoint (make swagger) Tests: preview-equals-write (JSON+TOML), masking, entry_exists (create vs overwrite), no-side-effects (no write, no backup, mtime unchanged), absent/ malformed/denied degradation. go test -race ./internal/connect/... and ./internal/httpapi/... pass; golangci-lint v2 clean. Related #793 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): preview the exact change before writing in wizard + Connect modal Second slice of specs/078-connect-trust-preview US1 (FR-001,003,004): the Connect action in BOTH the standalone Connect modal and the onboarding wizard's ClientRow now fetches the preview first and shows an inline panel before any file is written: - target config path and, in a monospace additive ("+ will be added") block, the exact entry that will be written (pretty JSON, or TOML for Codex) with the API key masked; - plain-language trust copy ("Only this entry is added — everything else in the file stays untouched. A timestamped backup is created first."); - a masked-key line when contains_api_key so the user knows a credential is written; - an overwrite warning when an entry with the same name already exists, in which case confirming implies force=true; - graceful copy for the bridge/no-prior-file (access_state=absent) and unparseable (malformed) cases. Confirm proceeds with the connect (reusing the existing #799 post-connect backup-path line, untouched); Cancel dismisses the panel and writes nothing. A non-denial preview fetch error is surfaced inline; in the modal a denied read still resolves the Spec 075 access banner via checkAccess. Wizard connect-step telemetry (markConnectCompleted/Skipped) is unchanged. - types/api.ts: ConnectPreview; services/api.ts: getConnectPreview - ConnectModal.vue + OnboardingWizard.vue ClientRow: preview panel + flow - The existing #799 connect specs are updated to drive click → preview → confirm (Connect no longer writes on the first click); new connect-preview.spec.ts covers preview render, cancel-writes-nothing, confirm-proceeds, masked key, and overwrite→force for both surfaces. Full frontend suite: 33 files, 244 tests pass; vue-tsc clean; make build embeds the updated dist. npm-ci lockfile churn reverted (as in #799). Related #793 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(connect): stop leaking the API key into client configs; auth-aware carrier Security fix folded into Spec 078. Connect (and the new preview) previously appended ?apikey=<REST-admin key> to EVERY client entry via mcpURL(), but /mcp does not require auth by default (config.RequireMCPAuth defaults false; the admin context is granted unauthenticated). So connect leaked the REST-admin API key in plaintext into other apps' config files where it served no purpose. New behavior, threaded from config.RequireMCPAuth into connect.Service the same way listenAddr/apiKey are (server wiring + both CLI sites), via WithRequireMCPAuth: - require_mcp_auth=false (default): the entry carries NO credential — a clean http://host:port/mcp URL, no query, no headers. Existing configs keep working because /mcp accepts unauthenticated requests when protection is off. - require_mcp_auth=true: the credential is written via the carrier each client's real config schema supports (verified against vendor docs): * X-API-Key header — claude-code, vscode, cursor, windsurf, gemini, opencode (all support a "headers" object on their remote entry); * mcp-remote --header "X-API-Key:<key>" bridge arg — claude-desktop (no space after the colon: Claude Desktop/Cursor don't escape spaces in args and mangle the header — geelen/mcp-remote README; our keys are space-free); * ?apikey= query fallback — codex only, whose TOML HTTP transport takes header values indirectly via env-var names (http_headers / bearer_token_env_var) and cannot express a literal header value. Server-side ExtractToken accepts X-API-Key, Authorization: Bearer, and ?apikey= (in that order), so header and query authenticate identically. buildServerEntry now takes serverEntryParams{baseURL, credential}; mcpURL() is replaced by the credential-free baseURL() anchor. Entry MATCHING (findEntry*/findEquivalentJSONServerName/entryPointsToBridge) already keys on baseURL with an optional ?query, so it recognizes BOTH legacy ?apikey= entries and new clean entries — reconnect adopts/updates in place (no duplicate) and disconnect still finds legacy entries; a force-reconnect over a legacy entry upgrades it and drops the leaked key. Preview reflects reality: contains_api_key is true only when require_mcp_auth is on; the credential is masked in whichever carrier it uses (header value, bridge arg, or query) and the real key never appears in the preview payload. Tests: matrix over require_mcp_auth on/off × all 8 clients for both write and preview (preview==write shape, auth-off writes contain NO apikey/headers at all); per-client carrier assertions; legacy ?apikey= migration (recognized, upgraded in place, no duplicate, disconnect still works). go test -race ./internal/connect/... ./internal/httpapi/... pass; server/config/management pass; golangci-lint v2 clean. Related #793 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): carrier-agnostic credential copy in the connect preview Follows the connect security fix: the preview panel's masked-credential line now reads "This entry includes your API key (shown masked)…" instead of "The URL includes…", since with require_mcp_auth on the credential is carried in an X-API-Key header (or the mcp-remote --header bridge arg) for most clients and only falls back to the ?apikey= URL query for Codex. The line already renders only when contains_api_key is true, which the backend now sets only when require_mcp_auth is on — so the default (keyless) connect shows no credential line at all. Adds a frontend test for the default keyless case: contains_api_key=false ⇒ no credential line, and the previewed entry shows neither an apikey query nor a mask. Full frontend suite: 33 files, 245 tests pass; vue-tsc clean. Related #793 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(connect): live-config carrier + honest preview for edge cases Codex review of the Spec 078 US1 connect-preview branch surfaced one blocker and three preview/write-divergence gaps; this addresses them: - blocker: the HTTP connect service snapshotted require_mcp_auth (and listen/api_key) once at startup and was never rebuilt, while the /mcp auth middleware honors require_mcp_auth live per-request. Toggling auth off at runtime (wizard toggle or hot-reload) left the service embedding the real API key into client configs — the exact leak this branch closes. Add a live config-provider seam (WithConfigProvider) wired to runtime.Config() so preview and write always match the currently-enforced auth state. - OpenCode preview divergence: previewEntryExists checked only the exact server name, but OpenCode's write path adopts/normalizes an equivalent entry (incl. the legacy ?apikey= shape) under a different key. Preview now mirrors that via findEquivalentJSONServerName so entry_exists reflects the overwrite. - preview endpoint ignored server_name and always previewed "mcpproxy" while POST connect accepts a custom name. Honor an optional ?server_name= query so a caller previewing then connecting under a custom name does not diverge. - malformed config: the write parses the same bytes and would fail, but the preview copy promised it "still writes only the entry" and left Connect enabled. Make the copy honest and disable Connect for malformed configs. Tests: live-toggle credential (auth off->on without rebuild), OpenCode equivalent-entry preview, server_name honoring, and malformed confirm-disabled. go test -race ./internal/connect/... ./internal/httpapi/... green; frontend 246 tests + vue-tsc clean; golangci-lint v2 ./... 0 issues; swagger regenerated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web): make the first connect click explicitly commitment-free The preview only appeared AFTER clicking "Connect" — the exact button hesitant users avoid, fearing the click immediately modifies their config. Rename the row action to "Review & connect" and say up front in the modal subtitle and wizard intro that nothing is written until the confirm step, so the safety of the first click is visible before it happens (Spec 078 US1 trust copy). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(web): align setup.md and test prose with the Review & connect flow Codex review follow-up: docs/setup.md described the Claude Desktop bridge registration as one-click "Connect"; it is now review-then- confirm. Test comments updated to match the renamed row action. Spec 046/075 texts intentionally left untouched — they are point-in- time records (046 FR-005 already required the diff preview this branch implements). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ss detector, deep_scan opt-in Reconcile the security docs with the post-077 code: - tool-scanner.md: remove the deleted legacy-TPA-rule coexistence section; the detect engine is now the sole in-process detector. Count the actually- registered checks — seven (four hard incl. phrase.injection, three soft) — and fix the front-matter, "The seven checks", and at-a-glance table. - security-scanner-plugins.md: drop the legacy-rules claim + six-check count from the tpa-descriptions row; document the security.deep_scan block as the primary config surface; remove the deprecated auto_scan_quarantined / scanner_fetch_package_source docs; note the deep-scan gate + enable hint. - docker-isolation.md (+ top-level copy): the sandbox/none Docker-scanner skip no longer downgrades to security_scan.status:"degraded" (FR-008); it surfaces via the always-emitted deep_scan descriptor. Replace the deprecated scanner_disable_no_new_privileges instruction with deep_scan.disable_no_new_privileges. - security-quarantine.md: replace the deleted keyword-heuristic Detection Patterns table + Security Analysis JSON with the real quarantine-block shape and a pointer to the seven-check detect engine. - configuration.md: add the security.deep_scan config block + migration note. Related #793 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
US1 (#786), US2 (#792), US4 (#794), and US3/deepscan-fixes (this branch) are all merged. Check off every demonstrably-done task (verified against code + git log) including the T037-T039 docs sweep. Left unchecked: T005 (contract schemas were never copied to internal/security/scanner/testdata/ — tests validate behavior directly), and the final validation gates T040-T042. Tally: 38/42. Regenerate ROADMAP.md (077 now 38/42, in-flight). Related #793 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ilure) TestScanSummaryEnricherAdapterCarriesDeepScan passed its assertions but failed on windows-latest: t.TempDir cleanup cannot unlink config.db while the bbolt handle is open. setupTestStorage registers no close; add an explicit t.Cleanup in this test's seed helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dumbris
added a commit
that referenced
this pull request
Jul 2, 2026
…ne, connect-trust/upgrade-nudge progress (#803) check-github now passes with 0 errors: scanner-simplification epic complete (#786/#792/#793/#794 incl. deep-scan trust fixes + docs sweep); connect-trust US1 preview (#802) + backup visibility (#799) done; upgrade-nudge status/log slice (#798) split out as done with the banner+config remainder tracked separately; telemetry machine_id client (#796) and hygiene check-github (#800) done. Remaining warnings are the known windows-tray no-PR-evidence items. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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.
Summary
Spec 077 US3 — the opt-in deep scan layer. The deterministic offline baseline (US1, merged as #786 / 5b925e4) is always-on and drives the verdict; deep scan (Docker-based scanners + package source extraction) is now an opt-in layer that is off by default and never blocks or degrades the baseline verdict.
Supersedes the auto-closed stacked PR #789 (that PR was built on the original US1 head; it closed automatically when US1 was squash-merged to main). This branch has been rebased onto main and now includes the US1 Codex-round review fixes it was previously stacked behind.
What changed
security.deep_scanunified config block (internal/config/config.go) withenabled(default false),scannersallow-list,fetch_package_source,disable_no_new_privileges. Deprecated top-level keys (scanner_fetch_package_source,scanner_disable_no_new_privileges) are folded into it bymigrateDeepScanConfigon load (internal/config/loader.go); the removedauto_scan_quarantinedkey is ignored. Legacy keys are cleared so the migrated config serializes only the new surface.internal/security/scanner/engine.go):deepScanAllowed— the in-process baseline scanner is always allowed; Docker/deep scanners run only whendeep_scan.enabledis true (and, when set, on the per-scanner allow-list). Zero Docker invoked when off.DeepScanDescriptor(service.go/types.go): populated informational descriptor (enabled / ran / available / per-scanner failures) surfaced onScanSummaryandScanReport. A failed or unavailable deep scanner is reported here as a quiet note — it no longer downgrades a clean baseline to "degraded".degradeIfIncompleteCoverageis removed.deep_scan.enabled+fetch_package_source.Security.vue/ScanReport.vue/types/api.tssurface the opt-in deep scan and render deep-scan gaps as info (not blocking) + the deep-scan report banner wiring (review fix).DeepScanConfigsurface.Reconciliation notes (rebase onto final US1)
US1's final
service.go(Codex round-4) had already madeisBlockingFindingpurely tier-driven (f.Tier == TierHard) and removed the oldSummary.Critical > 0approval guard. This rebase keeps that behavior and layers US3'sDeepScanDescriptorpopulation on top — the old critical-count block is NOT reintroduced. US1'sDeepScanDescriptorplaceholder type is reconciled with US3's fuller version (single definition, no duplication).Verification (actual output on rebased head)
go build ./...→ exit 0go test ./internal/config/... ./internal/security/...→ allok(config, security, detect, detect/checks, patterns, scanner)make swagger-verify→ "✅ OpenAPI artifacts are up to date."scan-eval --gate --min-recall 0.90 --max-fp 0.05(detect_corpus_v1) → GATE PASSED: recall=1.0000, fp=0.0000golangci-lint run --config .github/.golangci.yml ./internal/...→ 0 issues🤖 Generated with Claude Code