diff --git a/.gitignore b/.gitignore index 60b8413..51fcab5 100644 --- a/.gitignore +++ b/.gitignore @@ -2,14 +2,13 @@ # Copy of default.config.yaml with real API keys — never commit /config.yml /config/ -# OAuth tokens, credentials, WhatsApp session data -/secrets/ -!/secrets/.gitkeep config.yml.bak blueprint/ /.understand-anything # ── Database & runtime data ─────────────────────────────────────────────────── /database/ +# Per-user container home dirs ({WD}/homes/{userid}) — instance data, not source +/homes/ # SQLite WAL-mode sidecar files (journal_mode=WAL) *.db-wal *.db-shm @@ -18,6 +17,9 @@ blueprint/ /logs/ /tmp/ /scripts/ +# Connector folders installed from the marketplace — instance data, like homes/ +# and database/, not source. See crates/skald-core/src/mcp/install.rs +/connectors/ # ── Rust build artifacts ────────────────────────────────────────────────────── /target/ @@ -58,6 +60,7 @@ scripts/.gitignore *.swo run-log.sh /backup.sh +/reset.sh debug/ # Honcho Docker secrets diff --git a/CLAUDE.md b/CLAUDE.md index 4d62374..0aa6062 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -100,8 +100,10 @@ Two rules keep the boundary real, and both are enforced by the compiler: The schema is split into two buckets (§5.1), and the split is the point: -- **`create_registry_tables`** — instance-wide, readable without any user key: `users`, `roles`, `llm_providers`, `llm_models`, `transcribe_models`, `tts_models`, `image_generate_models`, `plugins`, `approval_rules`, `tool_permission_groups`, `config`, `known_tools`, `llm_requests`, `mcp_catalog`, `mcp_global_servers` + `mcp_global_access`, `role_capabilities`, `shared_folders` + `shared_folder_members`. The MCP four back the Connectors model (§7/§14/§15 — see its own section). The last two (accessor `db/shared_folders.rs`) are the **membership** of the on-disk shared folders (§6): a junction table so a member can be read-only (`can_write`) and so the container mount topology + the `shared/{X}` fs routing both query it. FK `shared_folder_members.user_id → users(id)` is registry→registry (same file), which is allowed — unlike an owner→registry key. -- **`create_owner_tables`** — one owner's content, **identical schema in every file that has it**: `chat_sessions`, `chat_sessions_stack`, `chat_history`, `chat_llm_tools`, `chat_summaries`, `session_scratchpad`, `session_mcp_grants`, `stack_mcp_grants`, `scheduled_jobs`, `job_runs`, `mcp_user_servers`, `mcp_events`, `sources`, `secrets`, `projects`, `project_tickets`, `llm_request_payloads`, `memory_docs` (+ FTS5 `memory_docs_fts`). `mcp_user_servers` (a user's activated per-user connectors) carries `catalog_name` as a **bare `TEXT` snapshot** of `mcp_catalog.name`, never a FK — an owner→registry key would fail every INSERT. Because `memory_docs` is an owner table, one definition backs **private** memory in each `{userid}.db` and **shared** memory in `system.db` (the household owner) — see the memory namespace note below. +- **`create_registry_tables`** — instance-wide, readable without any user key: `users`, `roles`, `llm_providers`, `llm_models`, `transcribe_models`, `tts_models`, `image_generate_models`, `plugins`, `approval_rules`, `tool_permission_groups`, `config`, `known_tools`, `llm_requests`, `mcp_catalog`, `mcp_global_servers` + `mcp_global_access`, `oauth_providers`, `role_capabilities`, `shared_folders` + `shared_folder_members`. The MCP tables back the Connectors model (§7/§14/§15 — see its own section); `oauth_providers` (accessor `db/oauth_providers.rs`) holds one row per identity provider (Google…) — endpoints + `client_id`/`client_secret` + `redirect_uri`, admin-owned household secrets (§4/§15b), never a per-user token. The last two (accessor `db/shared_folders.rs`) are the **membership** of the on-disk shared folders (§6): a junction table so a member can be read-only (`can_write`) and so the container mount topology + the `shared/{X}` fs routing both query it. FK `shared_folder_members.user_id → users(id)` is registry→registry (same file), which is allowed — unlike an owner→registry key. +- **`create_owner_tables`** — one owner's content, **identical schema in every file that has it**: `chat_sessions`, `chat_sessions_stack`, `chat_history`, `chat_llm_tools`, `chat_summaries`, `session_scratchpad`, `session_mcp_grants`, `stack_mcp_grants`, `scheduled_jobs`, `job_runs`, `mcp_user_servers`, `mcp_events`, `sources`, `secrets`, `projects`, `project_tickets`, `llm_request_payloads`, `memory_docs` (+ FTS5 `memory_docs_fts`). `mcp_user_servers` (a user's activated per-user connectors) carries `catalog_name` as a **bare `TEXT` snapshot** of `mcp_catalog.name`, never a FK — an owner→registry key would fail every INSERT; for an OAuth connector it also snapshots `oauth_provider` + `deliver_json`, and its `api_key` column holds the refresh token (in the SQLCipher-encrypted file, so no column crypto). Because `memory_docs` is an owner table, one definition backs **private** memory in each `{userid}.db` and **shared** memory in `system.db` (the household owner) — see the memory namespace note below. + +Schema is greenfield (no migrations, §0), but a purely **additive** column lands on an existing DB in place: `db::ensure_column` runs `ALTER TABLE … ADD COLUMN` and swallows the "duplicate column" error, a no-op on a fresh DB where the `CREATE TABLE` already has the column. Used for the OAuth columns on `mcp_catalog` / `mcp_user_servers` so a dev box need not be wiped for an additive change (a full recreate is still valid). **No foreign key in the owner bucket may point at a registry table.** SQLite cannot enforce a key across files, not even through `ATTACH`, and sqlx turns on `PRAGMA foreign_keys`: the `CREATE TABLE` succeeds and every `INSERT` fails. `db::tests::owner_tables_stand_alone_with_foreign_keys_on` enforces this by running the owner schema against a database holding nothing else, then inserting a row into each table. Two keys crossed and were fixed: `chat_history.model_db_id` (dropped — write-only, and `llm_requests.model_name` already records the model) and `project_tickets.job_id` (fixed by moving `projects`/`project_tickets` into the owner bucket). @@ -145,11 +147,20 @@ MCP servers are surfaced to users as **"Connectors"** (UI naming; `mcp`/schema s **Authorization is a capability on the role, not `if role==admin`** (§0.1/§14 — `db/role_capabilities.rs`): `mcp.register_remote` + `mcp.register_local_from_catalog` are self-service (seeded on every new role by `roles::create` via `seed_defaults`); `mcp.register_local_script` + `mcp.manage_catalog` are admin-only. `admin` holds every capability by construction (short-circuit in `has()`). API handlers gate through `require_cap`. -**Tables** (see DB section) — registry: `mcp_catalog` (admin-vetted templates; holds only the *schema* of what an activation must supply, never live creds), `mcp_global_servers` + `mcp_global_access`, `role_capabilities`. Owner: `mcp_user_servers` (per-user activations; `api_key` encrypted at rest, `catalog_name` a bare `TEXT` snapshot). +**Tables** (see DB section) — registry: `mcp_catalog` (admin-vetted templates; holds only the *schema* of what an activation must supply, never live creds — plus, for OAuth, `oauth_provider` + `oauth_scopes_json` + `deliver_json`), `mcp_global_servers` + `mcp_global_access`, `oauth_providers` (per-provider client creds), `role_capabilities`. Owner: `mcp_user_servers` (per-user activations; `api_key` encrypted at rest — the refresh token for an OAuth one — `catalog_name`/`oauth_provider`/`deliver_json` bare `TEXT` snapshots). -**Endpoints** (`src/frontend/api/mcp.rs`, mounted in `api/mod.rs`) — admin: `/mcp/catalog` (GET/POST/DELETE), `/mcp/global` (list/enable/delete + `/{id}/access` GET/PUT). User: `/mcp/available`, `/mcp/activate`, `/mcp/activated` (+ DELETE `/{id}` to deactivate). `connectors.js` (``) renders the user view (activate/deactivate + granted globals) always, and the admin view (catalog + global + per-server access grants) when `role_id === 'admin'`. +**Endpoints** (`src/frontend/api/mcp.rs`, mounted in `api/mod.rs`) — admin: `/mcp/catalog` (GET/POST/DELETE), `/mcp/global` (list/enable/delete + `/{id}/access` GET/PUT), `/mcp/providers` (GET/POST + DELETE `/{name}` — OAuth provider creds, secret never returned to the browser). User: `/mcp/available`, `/mcp/activate`, `/mcp/activated` (+ DELETE `/{id}` to deactivate), `/mcp/oauth/start` + `/mcp/oauth/complete` (the §15 login). `connectors.js` (``) renders the user view (activate/deactivate + granted globals) always, plus the admin view (catalog + global + per-server access + a **Sign-in providers** modal) when `role_id === 'admin'`; `connector-detail.js` (``) is a connector's own page and hosts the OAuth login panel. -**Deferred:** interactive per-user auth (the §15 OAuth-callback / QR / SSH elicitation flow) — only `none`/`api_key` auth kinds are wired. No boot seed of catalog presets yet; the admin populates the catalog from the UI. +### OAuth per-user connectors (blueprint §15 — copy-paste flow) + +OAuth2 authorization-code + PKCE is wired for per-user connectors (Gmail is the first). The consent is a **human copy-paste**, not a headless action: no callback route into the (NAT'd, hostname-less) box, and no client secret on the public feed. + +- **Providers, not per-connector URLs.** The client is per-**provider** (one Google app covers Gmail/Calendar/Drive): `oauth_providers` holds `auth_url`/`token_url`/`client_id`/`client_secret`/`redirect_uri`/`extra_params`, admin-entered via the Sign-in-providers modal (Google preset fills all but the two secrets; `redirect_uri` = the static `oauth/show.html` page, `extra_params` = `access_type=offline`+`prompt=consent` so Google returns a refresh token). The manifest only names `auth.provider` + `auth.scopes` + `auth.deliver` — never URLs or secrets (feed is remote data, §14). +- **Flow** (`mcp/oauth.rs`): `activate` on an OAuth catalog entry persists a **pending** `mcp_user_servers` row (files installed, command wired, no token) and returns `needs_oauth` — it does **not** start the server. `/mcp/oauth/start` builds the consent URL (PKCE S256 + opaque `state`) and stashes the verifier in a RAM-only, TTL'd flow store keyed by `state`; the user approves in a browser, the provider lands the code on `oauth/show.html`, they paste it back. `/mcp/oauth/complete` exchanges code+verifier for a refresh token (`client_secret` sent server-side), stores it in the row's `api_key`, flips to `ready`, and starts the server. PKCE makes an intercepted code worthless; a restart drops in-flight flows (mirrors the RAM-only session model). +- **Credential delivery = env, nothing on disk.** The manifest's `deliver` (`{as,format,env}`, parsed as `mcp::DeliverSpec`) says how the token reaches the server. `user_row_spec_resolved` assembles the credential (`google_authorized_user` JSON = client creds from the provider + refresh token) and injects it as an env var (`GMAIL_CREDS_JSON`) on the `docker exec` — never a file, coherent with §2 (the tempted admin doesn't read `/proc`). The server reads it via `Credentials.from_authorized_user_info`. Ran both at OAuth-complete and at login-time per-user startup. +- **Google needs a Web-application client**: a Desktop client rejects an `https://` redirect (loopback only), so the `oauth/show.html` redirect must be registered on a **Web app** OAuth client, and exact-match under Authorized redirect URIs — `redirect_uri_mismatch` otherwise. + +**Deferred:** the other §15 interactive kinds (QR / SSH via elicitation) — `deliver.as=file` and non-Google providers are unimplemented paths that error clearly rather than half-work. No boot seed of catalog presets; the admin populates the catalog from the Marketplace. ## Sub-agent system @@ -251,7 +262,9 @@ All extend `LightElement` from `web/lib/base.js` (Lit). `ChatSession` (`web/lib/ | `agent-inbox.js` | `` | Pending approvals + clarifications from background sessions | | `approval-rules.js` | `` | Approval rule management | | `cron-jobs.js` | `` | Scheduled job management | -| `connectors.js` | `` | MCP Connectors: user activate/deactivate + granted globals; admin catalog + global-server + per-server access management (§7/§14/§15) | +| `connectors.js` | `` | MCP Connectors list (one row per connector): user activate/deactivate + granted globals; admin gets a **Sign-in providers** modal (OAuth client creds) + Catalog/Marketplace nav (§7/§14/§15) | +| `connector-detail.js` | `` | A connector's own page (`#connector?name=X`): env/secret form + Test, the **OAuth login panel** (sign in → paste code → complete, §15), global enable + per-user access grants | +| `shared/connector-common.js` | (helpers) | Shared Connectors vocabulary: `statusOf` (incl. `needs_login` for a pending OAuth row), `STATUS_LABEL`, schema normalization, `jf` fetch | | `llm-providers.js` | `` | LLM provider management | | `models-hub.js` | `` | Models hub landing (LLM / Transcription / Image) | | `models-llm.js` | `` | LLM model CRUD + drag-and-drop priority | diff --git a/crates/skald-core/src/approval/mod.rs b/crates/skald-core/src/approval/mod.rs index 58f5886..27b5cb2 100644 --- a/crates/skald-core/src/approval/mod.rs +++ b/crates/skald-core/src/approval/mod.rs @@ -338,12 +338,15 @@ impl ApprovalManager { /// shared memory is visible to everyone, so a write is a deliberate, human-confirmed /// act — the agent must not silently push one person's information into it. /// - `data/*` → **allow** (scratch/data workspace). - /// - `secrets/*` → **deny** (`@fs_any` denies reads *and* writes; a read would leak - /// the secret into the LLM context / history / WS stream, and `Deny` is - /// non-bypassable). The `/*` pattern also matches the `secrets` dir node itself, so - /// recursive `list_files`/`grep_files` rooted at it are covered. /// - `memory_search` → **allow**, path-less: it searches note *content* (arg `query`, /// not `path`), so it needs a tool-scoped rule rather than a path pattern. + /// + /// There is no `secrets/*` deny any more. It guarded an on-disk credential store + /// that no longer exists — connectors now take their credentials from the + /// activation form, held in the owner DB. Worse, it had quietly become wrong: + /// fs-tool paths are rooted at the caller's own home (§6), so `secrets/*` had + /// stopped meaning "the box's credential store" and started meaning "any folder a + /// user dared name `secrets`". pub async fn seed_fs_path_rules(&self) -> Result<()> { // (tool_pattern, path_pattern, action, note). `path_pattern = None` is a // tool-scoped rule that matches regardless of args. @@ -352,7 +355,6 @@ impl ApprovalManager { ("@fs_read", Some("shared-memory/*"), "allow", "auto-allow read shared-memory/"), ("@fs_write", Some("shared-memory/*"), "require", "require write shared-memory/"), ("@fs_any", Some("data/*"), "allow", "auto-allow data/"), - ("@fs_any", Some("secrets/*"), "deny", "deny secrets/ access"), ("memory_search", None, "allow", "allow memory_search"), ]; @@ -409,7 +411,9 @@ impl ApprovalManager { /// - the per-tool write `require` defaults (`note = 'default rule'`, no path) — fs /// gating now lives in the File System panel + the `*` catch-all; /// - the old `data/*` allow rows (`note = 'auto-allow data/ writes'`); - /// - the old `secrets` deny rows (`note = 'deny reading secrets/'`); + /// - both generations of `secrets/` deny row (`note = 'deny reading secrets/'` and + /// `'deny secrets/ access'`) — the on-disk secrets store is gone, and rooted at a + /// user's home the pattern had come to deny them any folder named `secrets`; /// - the old single `memory/*` allow row (`note = 'auto-allow memory/'`) — the memory /// namespace split into `user-memory/` + `shared-memory/`, so the `memory/*` pattern /// no longer routes and would otherwise linger as a stale allow on a disk `./memory/`. @@ -430,10 +434,13 @@ impl ApprovalManager { .await? .rows_affected(); - let n3 = sqlx::query("DELETE FROM approval_rules WHERE note = 'deny reading secrets/'") - .execute(self.db.as_ref()) - .await? - .rows_affected(); + let n3 = sqlx::query( + "DELETE FROM approval_rules + WHERE note IN ('deny reading secrets/', 'deny secrets/ access')", + ) + .execute(self.db.as_ref()) + .await? + .rows_affected(); let n4 = sqlx::query("DELETE FROM approval_rules WHERE note = 'auto-allow memory/'") .execute(self.db.as_ref()) @@ -1154,14 +1161,15 @@ mod tests { // Legacy per-tool fs rows are migrated away… let legacy: i64 = sqlx::query_scalar( "SELECT COUNT(*) FROM approval_rules - WHERE note IN ('default rule', 'auto-allow data/ writes', 'deny reading secrets/')", + WHERE note IN ('default rule', 'auto-allow data/ writes', 'deny reading secrets/', + 'deny secrets/ access')", ) .fetch_one(db.as_ref()) .await .unwrap(); assert_eq!(legacy, 0, "legacy fs rules should be removed by migration"); - // …and replaced by exactly the five @fs_* token rows (shared-memory has two: + // …and replaced by exactly the four @fs_* token rows (shared-memory has two: // read-allow and write-require). let fs_rows: i64 = sqlx::query_scalar( "SELECT COUNT(*) FROM approval_rules WHERE tool_pattern LIKE '@fs%'", @@ -1169,7 +1177,7 @@ mod tests { .fetch_one(db.as_ref()) .await .unwrap(); - assert_eq!(fs_rows, 5, "user-memory + shared-memory(r/w) + data + secrets @fs_* rules should be seeded"); + assert_eq!(fs_rows, 4, "user-memory + shared-memory(r/w) + data @fs_* rules should be seeded"); // Gate decisions through the real check() path. async fn decide(mgr: &ApprovalManager, tool: &str, path: &str) -> GateResult { @@ -1186,9 +1194,10 @@ mod tests { assert!(matches!(decide(&mgr, "read_file", "data/x.txt").await, GateResult::Allow)); // memory_search is allowed by a path-less tool rule (it has `query`, not `path`). assert!(matches!(decide(&mgr, "memory_search", "ignored").await, GateResult::Allow)); - // Improvement over legacy: secrets *writes* are now denied too, not just reads. - assert!(matches!(decide(&mgr, "write_file", "secrets/key").await, GateResult::Deny)); - assert!(matches!(decide(&mgr, "read_file", "secrets/key").await, GateResult::Deny)); + // The on-disk secrets store is gone, and with it its blanket deny: `secrets/` + // is now an ordinary path in the caller's own home, gated by the catch-all + // like any other. Denying it would deny a user their own folder. + assert!(matches!(decide(&mgr, "read_file", "secrets/key").await, GateResult::Require)); // Unmatched write falls through to the `*` catch-all. assert!(matches!(decide(&mgr, "write_file", "src/main.rs").await, GateResult::Require)); // Non-filesystem tool: unaffected by @fs_* rules, gated by catch-all. diff --git a/crates/skald-core/src/db/mcp_catalog.rs b/crates/skald-core/src/db/mcp_catalog.rs index f159ed3..2e01d03 100644 --- a/crates/skald-core/src/db/mcp_catalog.rs +++ b/crates/skald-core/src/db/mcp_catalog.rs @@ -26,14 +26,32 @@ pub struct McpCatalogRow { pub args_json: Option, pub env_json: Option, pub url: Option, - /// local_script: the vetted source path under `./scripts`. + /// local_script: the vetted entry file, as `/` under + /// `./connectors` (see [`crate::mcp::install`]). pub script_path: Option, - /// Names of the env/secret keys the activation UI must collect (never values). + /// JSON array of `{name,label,description,required,secret,example,default}` objects + /// describing the env/secret fields the activation UI must collect (never values). pub config_schema_json: Option, - /// 'none'|'api_key'|'oauth'|'qr'|'ssh_key'. Only 'none'/'api_key' are wired now. + /// 'none'|'api_key'|'oauth'|'qr'|'ssh_key'. pub auth_kind: String, + /// oauth: slug into `oauth_providers.name` (which app to consent to). + pub oauth_provider: Option, + /// oauth: JSON array of the scopes this connector requests at consent. + pub oauth_scopes_json: Option, + /// oauth: JSON `{as,format,env,path}` — how Skald delivers the obtained + /// credential to the connector's server process (§15). + pub deliver_json: Option, /// JSON array of role ids allowed to activate this; NULL = all roles (§15). pub role_filter: Option, + /// Shell command run before persisting an activation (verify-before-save). + pub verify_command: Option, + /// Script file the verify command references (e.g. `verify.py`), if any. + pub verify_script_path: Option, + /// Icon file *inside* `./connectors//`, if the feed shipped one. Stored + /// rather than derived because the manifest names its icons freely (`.png` for + /// one connector, `.svg` for the next), and the browser cannot guess. + pub icon_small_path: Option, + pub icon_large_path: Option, pub friendly_name: Option, pub description: Option, pub created_at: String, @@ -65,11 +83,21 @@ impl McpCatalogRow { Some(roles) => roles.iter().any(|r| r == role_id), } } + + /// The OAuth scopes this connector requests at consent, or empty when it is not + /// an OAuth connector. + pub fn oauth_scopes(&self) -> Vec { + self.oauth_scopes_json.as_deref() + .and_then(|s| serde_json::from_str(s).ok()) + .unwrap_or_default() + } } const SELECT: &str = "SELECT id, name, scope, source, transport, command, args_json, env_json, url, \ - script_path, config_schema_json, auth_kind, role_filter, friendly_name, \ + script_path, config_schema_json, auth_kind, oauth_provider, oauth_scopes_json, \ + deliver_json, role_filter, verify_command, \ + verify_script_path, icon_small_path, icon_large_path, friendly_name, \ description, created_at \ FROM mcp_catalog"; @@ -122,7 +150,14 @@ pub struct UpsertCatalog<'a> { pub script_path: Option<&'a str>, pub config_schema_json: Option, pub auth_kind: &'a str, + pub oauth_provider: Option<&'a str>, + pub oauth_scopes_json: Option, + pub deliver_json: Option, pub role_filter: Option, + pub verify_command: Option<&'a str>, + pub verify_script_path: Option<&'a str>, + pub icon_small_path: Option<&'a str>, + pub icon_large_path: Option<&'a str>, pub friendly_name: Option<&'a str>, pub description: Option<&'a str>, } @@ -131,8 +166,10 @@ pub async fn upsert(pool: &SqlitePool, e: UpsertCatalog<'_>) -> Result { let row = sqlx::query_as::<_, (i64,)>( "INSERT INTO mcp_catalog (name, scope, source, transport, command, args_json, env_json, url, - script_path, config_schema_json, auth_kind, role_filter, friendly_name, description) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14) + script_path, config_schema_json, auth_kind, oauth_provider, oauth_scopes_json, + deliver_json, role_filter, verify_command, + verify_script_path, icon_small_path, icon_large_path, friendly_name, description) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18, ?19, ?20, ?21) ON CONFLICT(name) DO UPDATE SET scope = excluded.scope, source = excluded.source, @@ -144,7 +181,18 @@ pub async fn upsert(pool: &SqlitePool, e: UpsertCatalog<'_>) -> Result { script_path = excluded.script_path, config_schema_json = excluded.config_schema_json, auth_kind = excluded.auth_kind, + oauth_provider = excluded.oauth_provider, + oauth_scopes_json = excluded.oauth_scopes_json, + deliver_json = excluded.deliver_json, role_filter = excluded.role_filter, + verify_command = excluded.verify_command, + verify_script_path = excluded.verify_script_path, + -- Icons belong to whoever installed the files, not to whoever last + -- edited the row: COALESCE keeps them when an admin saves the catalog + -- form (which knows nothing about icons and would otherwise blank + -- them), while a reinstall still updates them. + icon_small_path = COALESCE(excluded.icon_small_path, mcp_catalog.icon_small_path), + icon_large_path = COALESCE(excluded.icon_large_path, mcp_catalog.icon_large_path), friendly_name = excluded.friendly_name, description = excluded.description RETURNING id", @@ -160,7 +208,14 @@ pub async fn upsert(pool: &SqlitePool, e: UpsertCatalog<'_>) -> Result { .bind(e.script_path) .bind(e.config_schema_json) .bind(e.auth_kind) + .bind(e.oauth_provider) + .bind(e.oauth_scopes_json) + .bind(e.deliver_json) .bind(e.role_filter) + .bind(e.verify_command) + .bind(e.verify_script_path) + .bind(e.icon_small_path) + .bind(e.icon_large_path) .bind(e.friendly_name) .bind(e.description) .fetch_one(pool) diff --git a/crates/skald-core/src/db/mcp_global_servers.rs b/crates/skald-core/src/db/mcp_global_servers.rs index f4036cb..ecc8af5 100644 --- a/crates/skald-core/src/db/mcp_global_servers.rs +++ b/crates/skald-core/src/db/mcp_global_servers.rs @@ -14,18 +14,22 @@ use sqlx::SqlitePool; #[derive(Debug, Clone, Serialize, sqlx::FromRow)] pub struct McpGlobalServerRow { - pub id: i64, - pub name: String, - pub catalog_name: Option, - pub transport: String, - pub command: Option, - pub args_json: Option, - pub env_json: Option, - pub url: Option, - pub api_key: Option, - pub friendly_name: Option, - pub description: Option, - pub enabled: bool, + pub id: i64, + pub name: String, + pub catalog_name: Option, + pub transport: String, + pub command: Option, + pub args_json: Option, + pub env_json: Option, + pub url: Option, + pub api_key: Option, + /// Snapshot of `mcp_catalog.verify_command` (NULL = no test). + pub verify_command: Option, + /// Absolute host path of the verify script, if any. + pub verify_script_path: Option, + pub friendly_name: Option, + pub description: Option, + pub enabled: bool, } impl McpGlobalServerRow { @@ -44,7 +48,7 @@ impl McpGlobalServerRow { const SELECT: &str = "SELECT id, name, catalog_name, transport, command, args_json, env_json, url, \ - api_key, friendly_name, description, enabled \ + api_key, verify_command, verify_script_path, friendly_name, description, enabled \ FROM mcp_global_servers"; // ── Reads ──────────────────────────────────────────────────────────────────── @@ -82,34 +86,39 @@ pub async fn get_by_name(pool: &SqlitePool, name: &str) -> Result { - pub name: &'a str, - pub catalog_name: Option<&'a str>, - pub transport: &'a str, - pub command: Option<&'a str>, - pub args_json: Option, - pub env_json: Option, - pub url: Option<&'a str>, - pub api_key: Option<&'a str>, - pub friendly_name: Option<&'a str>, - pub description: Option<&'a str>, + pub name: &'a str, + pub catalog_name: Option<&'a str>, + pub transport: &'a str, + pub command: Option<&'a str>, + pub args_json: Option, + pub env_json: Option, + pub url: Option<&'a str>, + pub api_key: Option<&'a str>, + pub verify_command: Option<&'a str>, + pub verify_script_path: Option<&'a str>, + pub friendly_name: Option<&'a str>, + pub description: Option<&'a str>, } pub async fn upsert(pool: &SqlitePool, p: UpsertGlobal<'_>) -> Result { let row = sqlx::query_as::<_, (i64,)>( "INSERT INTO mcp_global_servers - (name, catalog_name, transport, command, args_json, env_json, url, api_key, friendly_name, description, enabled) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, 1) + (name, catalog_name, transport, command, args_json, env_json, url, api_key, + verify_command, verify_script_path, friendly_name, description, enabled) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, 1) ON CONFLICT(name) DO UPDATE SET - catalog_name = excluded.catalog_name, - transport = excluded.transport, - command = excluded.command, - args_json = excluded.args_json, - env_json = excluded.env_json, - url = excluded.url, - api_key = excluded.api_key, - friendly_name = excluded.friendly_name, - description = excluded.description, - enabled = 1 + catalog_name = excluded.catalog_name, + transport = excluded.transport, + command = excluded.command, + args_json = excluded.args_json, + env_json = excluded.env_json, + url = excluded.url, + api_key = excluded.api_key, + verify_command = excluded.verify_command, + verify_script_path = excluded.verify_script_path, + friendly_name = excluded.friendly_name, + description = excluded.description, + enabled = 1 RETURNING id", ) .bind(p.name) @@ -120,6 +129,8 @@ pub async fn upsert(pool: &SqlitePool, p: UpsertGlobal<'_>) -> Result { .bind(p.env_json) .bind(p.url) .bind(p.api_key) + .bind(p.verify_command) + .bind(p.verify_script_path) .bind(p.friendly_name) .bind(p.description) .fetch_one(pool) diff --git a/crates/skald-core/src/db/mcp_user_servers.rs b/crates/skald-core/src/db/mcp_user_servers.rs index d8dbd37..15b92cc 100644 --- a/crates/skald-core/src/db/mcp_user_servers.rs +++ b/crates/skald-core/src/db/mcp_user_servers.rs @@ -15,24 +15,34 @@ use sqlx::SqlitePool; #[derive(Debug, Clone, Serialize, sqlx::FromRow)] pub struct McpUserServerRow { - pub id: i64, - pub name: String, + pub id: i64, + pub name: String, /// Bare snapshot of the originating `mcp_catalog.name`; NULL for a /// self-registered remote. - pub catalog_name: Option, + pub catalog_name: Option, /// 'remote' | 'local_script'. - pub source: String, - pub transport: String, - pub command: Option, - pub args_json: Option, - pub env_json: Option, - pub url: Option, - pub api_key: Option, + pub source: String, + pub transport: String, + pub command: Option, + pub args_json: Option, + pub env_json: Option, + pub url: Option, + /// Per-user secret. For an OAuth connector this holds the refresh token; empty + /// until the OAuth flow completes (`auth_state='pending'`). + pub api_key: Option, + /// oauth: snapshot of the catalog's `oauth_provider` (which app issued the token). + pub oauth_provider: Option, + /// oauth: snapshot of the catalog's delivery spec `{as,format,env,path}`. + pub deliver_json: Option, /// Container path of the copied script, for a `local_script`. - pub script_rel_path: Option, - /// 'pending' | 'ready' — the interactive-auth gate ('ready' while api-key). - pub auth_state: String, - pub enabled: bool, + pub script_rel_path: Option, + /// Snapshot of `mcp_catalog.verify_command` (NULL = no test). + pub verify_command: Option, + /// Container path of the verify script, if any. + pub verify_script_rel_path: Option, + /// 'pending' | 'ready' — the verify-before-save gate. + pub auth_state: String, + pub enabled: bool, } impl McpUserServerRow { @@ -47,11 +57,19 @@ impl McpUserServerRow { .and_then(|s| serde_json::from_str(s).ok()) .unwrap_or_default() } + + /// The credential delivery spec (`{as,format,env,path}`), if this is an OAuth + /// connector that snapshotted one. + pub fn deliver(&self) -> Option { + self.deliver_json.as_deref() + .and_then(|s| serde_json::from_str(s).ok()) + } } const SELECT: &str = "SELECT id, name, catalog_name, source, transport, command, args_json, env_json, url, \ - api_key, script_rel_path, auth_state, enabled \ + api_key, oauth_provider, deliver_json, script_rel_path, verify_command, \ + verify_script_rel_path, auth_state, enabled \ FROM mcp_user_servers"; // ── Reads ──────────────────────────────────────────────────────────────────── @@ -93,24 +111,30 @@ pub async fn get_by_name(pool: &SqlitePool, name: &str) -> Result { - pub name: &'a str, - pub catalog_name: Option<&'a str>, - pub source: &'a str, - pub transport: &'a str, - pub command: Option<&'a str>, - pub args_json: Option, - pub env_json: Option, - pub url: Option<&'a str>, - pub api_key: Option<&'a str>, - pub script_rel_path: Option<&'a str>, - pub auth_state: &'a str, + pub name: &'a str, + pub catalog_name: Option<&'a str>, + pub source: &'a str, + pub transport: &'a str, + pub command: Option<&'a str>, + pub args_json: Option, + pub env_json: Option, + pub url: Option<&'a str>, + pub api_key: Option<&'a str>, + pub oauth_provider: Option<&'a str>, + pub deliver_json: Option, + pub script_rel_path: Option<&'a str>, + pub verify_command: Option<&'a str>, + pub verify_script_rel_path: Option<&'a str>, + pub auth_state: &'a str, } pub async fn insert(pool: &SqlitePool, s: InsertUserServer<'_>) -> Result { let id = sqlx::query( "INSERT INTO mcp_user_servers - (name, catalog_name, source, transport, command, args_json, env_json, url, api_key, script_rel_path, auth_state, enabled) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, 1)", + (name, catalog_name, source, transport, command, args_json, env_json, url, api_key, + oauth_provider, deliver_json, script_rel_path, verify_command, verify_script_rel_path, + auth_state, enabled) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, 1)", ) .bind(s.name) .bind(s.catalog_name) @@ -121,7 +145,11 @@ pub async fn insert(pool: &SqlitePool, s: InsertUserServer<'_>) -> Result { .bind(s.env_json) .bind(s.url) .bind(s.api_key) + .bind(s.oauth_provider) + .bind(s.deliver_json) .bind(s.script_rel_path) + .bind(s.verify_command) + .bind(s.verify_script_rel_path) .bind(s.auth_state) .execute(pool) .await? @@ -129,6 +157,17 @@ pub async fn insert(pool: &SqlitePool, s: InsertUserServer<'_>) -> Result { Ok(id) } +/// Stores a freshly-obtained OAuth refresh token and flips the connector to +/// `ready`, in one write — the completion of the §15 login flow. +pub async fn set_oauth_token(pool: &SqlitePool, id: i64, refresh_token: &str) -> Result<()> { + sqlx::query("UPDATE mcp_user_servers SET api_key = ?1, auth_state = 'ready' WHERE id = ?2") + .bind(refresh_token) + .bind(id) + .execute(pool) + .await?; + Ok(()) +} + pub async fn set_enabled(pool: &SqlitePool, id: i64, enabled: bool) -> Result<()> { sqlx::query("UPDATE mcp_user_servers SET enabled = ?1 WHERE id = ?2") .bind(enabled as i64) diff --git a/crates/skald-core/src/db/mod.rs b/crates/skald-core/src/db/mod.rs index 5c6aca0..7e699a6 100644 --- a/crates/skald-core/src/db/mod.rs +++ b/crates/skald-core/src/db/mod.rs @@ -17,6 +17,7 @@ pub mod mcp_global_access; pub mod mcp_global_servers; pub mod mcp_user_servers; pub mod memory_docs; +pub mod oauth_providers; pub mod plugins; pub mod role_capabilities; pub mod roles; @@ -155,6 +156,21 @@ pub async fn open_user_pool(path: &Path, key: Option<&Dek>) -> Result Result<()> { + let sql = format!("ALTER TABLE {table} ADD COLUMN {column} {decl}"); + if let Err(e) = sqlx::query(sqlx::AssertSqlSafe(sql)).execute(pool).await { + if !e.to_string().contains("duplicate column name") { + return Err(e.into()); + } + } + Ok(()) +} + // ── Registry tables ─────────────────────────────────────────────────────────── // // Instance-wide, readable without any user key: the directory you must open @@ -442,10 +458,17 @@ async fn create_registry_tables(pool: &SqlitePool) -> Result<()> { args_json TEXT, env_json TEXT, url TEXT, - script_path TEXT, -- local_script: source under ./scripts - config_schema_json TEXT, -- names of env/secret keys the UI must collect + script_path TEXT, -- local_script: entry file, as / under ./connectors + config_schema_json TEXT, -- env[] entries (objects) the UI must collect auth_kind TEXT NOT NULL DEFAULT 'none', -- 'none'|'api_key'|'oauth'|'qr'|'ssh_key' + oauth_provider TEXT, -- oauth: slug into oauth_providers.name + oauth_scopes_json TEXT, -- oauth: JSON array of scopes requested at consent + deliver_json TEXT, -- oauth: {as,format,env,path} credential delivery spec role_filter TEXT, -- JSON array of role ids; NULL = all + verify_command TEXT, -- shell command run before persisting an activation + verify_script_path TEXT, -- script file the verify command references, if any + icon_small_path TEXT, -- icon file inside ./connectors//, if the feed shipped one + icon_large_path TEXT, friendly_name TEXT, description TEXT, created_at TEXT NOT NULL DEFAULT (datetime('now')) @@ -453,6 +476,10 @@ async fn create_registry_tables(pool: &SqlitePool) -> Result<()> { ) .execute(pool) .await?; + // OAuth columns are additive (§15) — reach an already-created catalog in place. + ensure_column(pool, "mcp_catalog", "oauth_provider", "TEXT").await?; + ensure_column(pool, "mcp_catalog", "oauth_scopes_json", "TEXT").await?; + ensure_column(pool, "mcp_catalog", "deliver_json", "TEXT").await?; // Concrete globally-active connectors (shared, stateless — web-search etc.). // They run on the HOST. The global secret (admin's API key) is fine here: @@ -460,19 +487,21 @@ async fn create_registry_tables(pool: &SqlitePool) -> Result<()> { // FK (both in this file) — allowed. sqlx::query( "CREATE TABLE IF NOT EXISTS mcp_global_servers ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - name TEXT NOT NULL UNIQUE, - catalog_name TEXT REFERENCES mcp_catalog(name), - transport TEXT NOT NULL DEFAULT 'stdio', - command TEXT, - args_json TEXT, - env_json TEXT, - url TEXT, - api_key TEXT, - friendly_name TEXT, - description TEXT, - enabled INTEGER NOT NULL DEFAULT 1, - created_at TEXT NOT NULL DEFAULT (datetime('now')) + id INTEGER PRIMARY KEY AUTOINCREMENT, + name TEXT NOT NULL UNIQUE, + catalog_name TEXT REFERENCES mcp_catalog(name), + transport TEXT NOT NULL DEFAULT 'stdio', + command TEXT, + args_json TEXT, + env_json TEXT, + url TEXT, + api_key TEXT, + verify_command TEXT, -- snapshot of mcp_catalog.verify_command + verify_script_path TEXT, -- absolute host path of the verify script, if any + friendly_name TEXT, + description TEXT, + enabled INTEGER NOT NULL DEFAULT 1, + created_at TEXT NOT NULL DEFAULT (datetime('now')) )", ) .execute(pool) @@ -503,6 +532,32 @@ async fn create_registry_tables(pool: &SqlitePool) -> Result<()> { .execute(pool) .await?; + // OAuth providers for per-user connectors (blueprint §15). One row per identity + // provider (Google, …), referenced by name from `mcp_catalog.oauth_provider`. A + // single app covers every service of that provider (Gmail, Calendar, Drive) — + // client credentials are keyed on the provider, scopes on the connector. + // + // Registry table (`system.db`): `client_secret` is a household/global secret the + // admin owns (§4/§15b), not a per-user one, so it belongs here in the admin- + // readable file. The per-user refresh tokens each activation obtains never land + // here — they go, encrypted, into the user's `mcp_user_servers.api_key`. + sqlx::query( + "CREATE TABLE IF NOT EXISTS oauth_providers ( + name TEXT PRIMARY KEY, -- slug referenced by mcp_catalog.oauth_provider + display_name TEXT NOT NULL, + auth_url TEXT NOT NULL, -- authorization endpoint + token_url TEXT NOT NULL, -- token endpoint + client_id TEXT NOT NULL, + client_secret TEXT NOT NULL, + redirect_uri TEXT NOT NULL, -- copy-paste landing page (oauth/show.html) + extra_params TEXT, -- JSON of extra auth params (access_type, prompt, …) + created_at TEXT NOT NULL DEFAULT (datetime('now')), + updated_at TEXT NOT NULL DEFAULT (datetime('now')) + )", + ) + .execute(pool) + .await?; + Ok(()) } @@ -742,24 +797,31 @@ pub async fn create_owner_tables(pool: &SqlitePool) -> Result<()> { // copied into the bind-mounted home (`script_rel_path`). sqlx::query( "CREATE TABLE IF NOT EXISTS mcp_user_servers ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - name TEXT NOT NULL UNIQUE, - catalog_name TEXT, -- bare ref to mcp_catalog.name; NULL = self-registered remote - source TEXT NOT NULL, -- 'remote' | 'local_script' - transport TEXT NOT NULL DEFAULT 'stdio', - command TEXT, - args_json TEXT, - env_json TEXT, - url TEXT, - api_key TEXT, -- per-user secret / OAuth refresh token - script_rel_path TEXT, -- container path for a local_script - auth_state TEXT NOT NULL DEFAULT 'ready', -- 'pending' | 'ready' - enabled INTEGER NOT NULL DEFAULT 1, - created_at TEXT NOT NULL DEFAULT (datetime('now')) + id INTEGER PRIMARY KEY AUTOINCREMENT, + name TEXT NOT NULL UNIQUE, + catalog_name TEXT, -- bare ref to mcp_catalog.name; NULL = self-registered remote + source TEXT NOT NULL, -- 'remote' | 'local_script' + transport TEXT NOT NULL DEFAULT 'stdio', + command TEXT, + args_json TEXT, + env_json TEXT, + url TEXT, + api_key TEXT, -- per-user secret / OAuth refresh token + oauth_provider TEXT, -- oauth: snapshot of catalog oauth_provider + deliver_json TEXT, -- oauth: snapshot of catalog credential delivery spec + script_rel_path TEXT, -- container path for a local_script + verify_command TEXT, -- snapshot of mcp_catalog.verify_command + verify_script_rel_path TEXT, -- container path of the verify script, if any + auth_state TEXT NOT NULL DEFAULT 'ready', -- 'pending' | 'ready' + enabled INTEGER NOT NULL DEFAULT 1, + created_at TEXT NOT NULL DEFAULT (datetime('now')) )", ) .execute(pool) .await?; + // OAuth columns are additive (§15) — reach an already-created table in place. + ensure_column(pool, "mcp_user_servers", "oauth_provider", "TEXT").await?; + ensure_column(pool, "mcp_user_servers", "deliver_json", "TEXT").await?; sqlx::query( "CREATE TABLE IF NOT EXISTS sources ( diff --git a/crates/skald-core/src/db/oauth_providers.rs b/crates/skald-core/src/db/oauth_providers.rs new file mode 100644 index 0000000..9f04d4f --- /dev/null +++ b/crates/skald-core/src/db/oauth_providers.rs @@ -0,0 +1,147 @@ +//! OAuth identity providers for per-user connectors (blueprint §15). +//! +//! Registry table in `system.db`: one row per provider (Google, …), keyed by a +//! stable slug that `mcp_catalog.oauth_provider` references. A single provider row +//! covers every service that provider exposes (Gmail, Calendar, Drive) — the +//! client credentials live here, the per-connector scopes in the catalog. +//! +//! `client_secret` is a household/global secret the admin owns (§4/§15b), so it is +//! fine in the admin-readable file. Per-user refresh tokens never land here. + +use std::collections::HashMap; + +use anyhow::Result; +use serde::Serialize; +use sqlx::SqlitePool; + +#[derive(Debug, Clone, Serialize, sqlx::FromRow)] +pub struct OauthProviderRow { + pub name: String, + pub display_name: String, + pub auth_url: String, + pub token_url: String, + pub client_id: String, + /// Never leaves the process for the browser — see [`OauthProviderView`]. + pub client_secret: String, + pub redirect_uri: String, + pub extra_params: Option, + pub created_at: String, + pub updated_at: String, +} + +impl OauthProviderRow { + /// Extra authorization-endpoint params (e.g. `access_type=offline`, + /// `prompt=consent`) merged into the consent URL. Google needs both to return a + /// refresh token; a provider that needs neither leaves this NULL. + pub fn extra(&self) -> HashMap { + self.extra_params.as_deref() + .and_then(|s| serde_json::from_str(s).ok()) + .unwrap_or_default() + } +} + +/// The provider as the admin UI renders it — **without** `client_secret`. The list +/// endpoint reaches the browser, and the secret has no business there. +#[derive(Debug, Clone, Serialize)] +pub struct OauthProviderView { + pub name: String, + pub display_name: String, + pub auth_url: String, + pub token_url: String, + pub client_id: String, + pub redirect_uri: String, + pub extra_params: Option, + /// So the admin sees a secret is set without the value crossing the wire. + pub has_client_secret: bool, +} + +impl From for OauthProviderView { + fn from(r: OauthProviderRow) -> Self { + OauthProviderView { + has_client_secret: !r.client_secret.is_empty(), + name: r.name, + display_name: r.display_name, + auth_url: r.auth_url, + token_url: r.token_url, + client_id: r.client_id, + redirect_uri: r.redirect_uri, + extra_params: r.extra_params, + } + } +} + +const SELECT: &str = + "SELECT name, display_name, auth_url, token_url, client_id, client_secret, \ + redirect_uri, extra_params, created_at, updated_at \ + FROM oauth_providers"; + +// ── Reads ──────────────────────────────────────────────────────────────────── + +pub async fn list(pool: &SqlitePool) -> Result> { + let rows = sqlx::query_as::<_, OauthProviderRow>(sqlx::AssertSqlSafe(format!("{SELECT} ORDER BY name"))) + .fetch_all(pool) + .await?; + Ok(rows) +} + +pub async fn get(pool: &SqlitePool, name: &str) -> Result> { + let row = sqlx::query_as::<_, OauthProviderRow>(sqlx::AssertSqlSafe(format!("{SELECT} WHERE name = ?"))) + .bind(name) + .fetch_optional(pool) + .await?; + Ok(row) +} + +// ── Writes ─────────────────────────────────────────────────────────────────── + +pub struct UpsertProvider<'a> { + pub name: &'a str, + pub display_name: &'a str, + pub auth_url: &'a str, + pub token_url: &'a str, + pub client_id: &'a str, + pub client_secret: &'a str, + pub redirect_uri: &'a str, + pub extra_params: Option<&'a str>, +} + +pub async fn upsert(pool: &SqlitePool, p: UpsertProvider<'_>) -> Result<()> { + sqlx::query( + "INSERT INTO oauth_providers + (name, display_name, auth_url, token_url, client_id, client_secret, redirect_uri, extra_params) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8) + ON CONFLICT(name) DO UPDATE SET + display_name = excluded.display_name, + auth_url = excluded.auth_url, + token_url = excluded.token_url, + client_id = excluded.client_id, + -- Keep the stored secret when the form submits an empty one: the admin + -- editing a provider's URLs should not have to re-paste the secret, + -- which the list view never gave back to the browser. + client_secret = CASE WHEN excluded.client_secret = '' + THEN oauth_providers.client_secret + ELSE excluded.client_secret END, + redirect_uri = excluded.redirect_uri, + extra_params = excluded.extra_params, + updated_at = datetime('now')", + ) + .bind(p.name) + .bind(p.display_name) + .bind(p.auth_url) + .bind(p.token_url) + .bind(p.client_id) + .bind(p.client_secret) + .bind(p.redirect_uri) + .bind(p.extra_params) + .execute(pool) + .await?; + Ok(()) +} + +pub async fn delete(pool: &SqlitePool, name: &str) -> Result<()> { + sqlx::query("DELETE FROM oauth_providers WHERE name = ?") + .bind(name) + .execute(pool) + .await?; + Ok(()) +} diff --git a/crates/skald-core/src/mcp/install.rs b/crates/skald-core/src/mcp/install.rs new file mode 100644 index 0000000..4554073 --- /dev/null +++ b/crates/skald-core/src/mcp/install.rs @@ -0,0 +1,181 @@ +//! On-disk layout of installed connectors (blueprint §7/§14). +//! +//! One folder per connector, `{WD}/connectors//`, holding exactly what the +//! marketplace served: the runtime files, the icons, and the `connector.json` the +//! admin accepted. It sits beside `homes/` and `shared/` because it belongs to the +//! **instance**, not to the checkout — `scripts/` was the wrong home for it, being +//! a source-tree directory that also carries hand-written dev scripts. +//! +//! Two consumers, and the split matters: +//! +//! - A **global** connector runs on the host, straight out of this folder. +//! - A **per-user** connector runs inside the user's container, so its runtime +//! files are copied into the bind-mounted home ([`install_into_home`]) — the only +//! durable zone (§6), so they survive a container recreate. +//! +//! `connector.json` is written but never read back: [`crate::db::mcp_catalog`] is +//! the only thing that drives a connect. The file is provenance — what was accepted, +//! and on what day — which is also what makes a later silent upstream change +//! detectable. Reading it at runtime would create a second source of truth that +//! diverges the moment the admin edits the catalog row. + +use std::path::{Path, PathBuf}; + +use anyhow::{Context, Result, bail}; + +use crate::container::{CONTAINER_HOME, HOMES_DIR}; + +/// Subdirectory of the working directory holding installed connector folders. +pub const CONNECTORS_DIR: &str = "connectors"; + +/// The manifest, saved verbatim at install time as provenance (never read back). +pub const MANIFEST_FILE: &str = "connector.json"; + +/// Where a per-user connector's files land inside the container, under the home +/// mount. `{CONTAINER_HOME}/.skald/mcp//`. +const IN_CONTAINER_MCP_SUBDIR: &str = ".skald/mcp"; + +/// The host directory holding `name`'s installed files. Does not check existence — +/// callers that need the files present say so themselves, with their own message. +pub fn connector_dir(name: &str) -> Result { + let wd = std::env::current_dir().context("failed to read working directory")?; + Ok(wd.join(CONNECTORS_DIR).join(name)) +} + +/// Splits a catalog `script_path` (`/`) into the connector folder and +/// the entry file's path *inside* it. +/// +/// The tail is kept whole rather than reduced to a basename: a connector may ship a +/// tree (`pkg/server.py`), and flattening it would break the import that made it a +/// tree in the first place. +pub fn split_script_path(script_path: &str) -> Result<(&str, &str)> { + match script_path.split_once('/') { + Some((folder, rel)) if !folder.is_empty() && !rel.is_empty() => Ok((folder, rel)), + _ => bail!("script_path `{script_path}` is not of the form `/`"), + } +} + +/// Whether a file is a host-side asset rather than something the runtime needs. +/// +/// Icons are for the browser and the manifest is provenance; neither has any job +/// inside a user's container, so they stay out of the home. The rule is extension- +/// based because the manifest names icons freely (`icon_sm.png`, `icon_lg.svg`); +/// if some future connector ever ships an image it genuinely needs at runtime, this +/// is the one place to reconsider. +pub fn is_host_asset(rel: &str) -> bool { + if rel == MANIFEST_FILE { + return true; + } + let ext = Path::new(rel) + .extension() + .and_then(|e| e.to_str()) + .unwrap_or_default() + .to_ascii_lowercase(); + matches!(ext.as_str(), "png" | "svg" | "jpg" | "jpeg" | "webp" | "gif" | "ico") +} + +/// The in-container path of a per-user connector's directory. +fn container_dir_for(runtime_name: &str) -> PathBuf { + Path::new(CONTAINER_HOME).join(IN_CONTAINER_MCP_SUBDIR).join(runtime_name) +} + +/// The host path of a per-user connector's directory, inside the bind-mounted home. +fn home_dir_for(user_id: &str, runtime_name: &str) -> Result { + let wd = std::env::current_dir().context("failed to read working directory")?; + Ok(wd + .join(HOMES_DIR) + .join(user_id) + .join(IN_CONTAINER_MCP_SUBDIR) + .join(runtime_name)) +} + +/// Copies the runtime files of the installed connector `folder` into `user_id`'s +/// home under `.skald/mcp//`, and returns the directory's path +/// **inside** the container. +/// +/// The whole tree is copied, minus host assets ([`is_host_asset`]) — which is what +/// finally gets a connector's `requirements.txt` and its multi-file trees into the +/// container, where copying a single entry file never did. +/// +/// Returns `Ok(None)` when `folder` was never installed on this box, so a caller +/// that does not actually need the files (a catalog entry pointing at nothing, a +/// connector with no verify step) can carry on. Idempotent: re-running overwrites. +pub fn install_into_home( + user_id: &str, + runtime_name: &str, + folder: &str, +) -> Result> { + let src = connector_dir(folder)?; + if !src.is_dir() { + return Ok(None); + } + let dest = home_dir_for(user_id, runtime_name)?; + std::fs::create_dir_all(&dest) + .with_context(|| format!("failed to create {}", dest.display()))?; + copy_runtime_files(&src, &dest, Path::new(""))?; + Ok(Some(container_dir_for(runtime_name))) +} + +/// Recursively copies `src` into `dest`, skipping host assets. `rel` tracks the +/// path relative to the connector root so [`is_host_asset`] sees the same string +/// the manifest declared. +fn copy_runtime_files(src: &Path, dest: &Path, rel: &Path) -> Result<()> { + for entry in std::fs::read_dir(src).with_context(|| format!("cannot read {}", src.display()))? { + let entry = entry?; + let name = entry.file_name(); + let child_rel = rel.join(&name); + let from = entry.path(); + let to = dest.join(&name); + + if entry.file_type()?.is_dir() { + std::fs::create_dir_all(&to) + .with_context(|| format!("failed to create {}", to.display()))?; + copy_runtime_files(&from, &to, &child_rel)?; + continue; + } + if is_host_asset(&child_rel.to_string_lossy()) { + continue; + } + std::fs::copy(&from, &to) + .with_context(|| format!("failed to copy {}", child_rel.display()))?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn splits_a_script_path_into_folder_and_tail() { + assert_eq!(split_script_path("gmail/server.py").unwrap(), ("gmail", "server.py")); + // A tree keeps its shape — the tail is not reduced to a basename. + assert_eq!( + split_script_path("whatsapp/pkg/index.js").unwrap(), + ("whatsapp", "pkg/index.js") + ); + for bad in ["server.py", "", "gmail/", "/server.py"] { + assert!(split_script_path(bad).is_err(), "should have rejected `{bad}`"); + } + } + + /// Icons and the manifest are host-side only: they must never reach a user's + /// container, while everything the server actually runs on must. + #[test] + fn host_assets_are_icons_and_the_manifest() { + for asset in ["connector.json", "icon_sm.png", "icon_lg.svg", "a/b/logo.WEBP"] { + assert!(is_host_asset(asset), "`{asset}` should be a host asset"); + } + for runtime in ["server.py", "requirements.txt", "pkg/index.js", "verify.py"] { + assert!(!is_host_asset(runtime), "`{runtime}` should reach the container"); + } + } + + #[test] + fn container_dir_hangs_off_the_home_mount() { + assert_eq!( + container_dir_for("gmail"), + PathBuf::from("/root/.skald/mcp/gmail") + ); + } +} diff --git a/crates/skald-core/src/mcp/mod.rs b/crates/skald-core/src/mcp/mod.rs index 7795aff..f4b859d 100644 --- a/crates/skald-core/src/mcp/mod.rs +++ b/crates/skald-core/src/mcp/mod.rs @@ -24,10 +24,16 @@ pub use mcp_client::{ use mcp_client::McpTransport; +pub mod install; mod logs; +pub mod oauth; mod provider; +pub mod verify; +pub use install::{CONNECTORS_DIR, MANIFEST_FILE, connector_dir, install_into_home, split_script_path}; +pub use oauth::DeliverSpec; pub use provider::{McpProvider, UserMcpView}; +pub use verify::{VerifyReport, VerifyTarget, apply_placeholders, run_verify}; const SERVER_START_TIMEOUT_SECS: u64 = 120; @@ -409,10 +415,35 @@ fn apply_key_placeholder( ) -> (Option, Option) { match (url, api_key) { (Some(u), Some(k)) if u.contains("{key}") => (Some(u.replace("{key}", &k)), None), + // Unified {SECRET:} placeholder (e.g. Tavily's + // `?tavilyApiKey={SECRET:tavilyApiKey}`). Any SECRET token in a URL is + // the api_key for a remote connector — a URL never carries the user's + // other secrets — so we substitute every occurrence. + (Some(u), Some(k)) if u.contains("{SECRET:") => (Some(substitute_secret_tokens(&u, &k)), None), (u, k) => (u, k), } } +/// Replaces every `{SECRET:…}` token in `text` with `value`. Used for the +/// api-key-in-URL case; other placeholders are left untouched. +fn substitute_secret_tokens(text: &str, value: &str) -> String { + let mut out = String::with_capacity(text.len()); + let mut rest = text; + while let Some(open) = rest.find("{SECRET:") { + out.push_str(&rest[..open]); + let after = &rest[open..]; + if let Some(close) = after.find('}') { + out.push_str(value); + rest = &after[close + 1..]; + } else { + out.push_str(after); + break; + } + } + out.push_str(rest); + out +} + /// Builds a spec for a globally-active connector — host transport (`launch_in` /// = None), so it runs in the Skald process, not in any container (§7). pub fn global_row_spec(row: &crate::db::mcp_global_servers::McpGlobalServerRow) -> McpServerSpec { @@ -460,6 +491,51 @@ pub fn user_row_spec( } } +/// Like [`user_row_spec`], but for an OAuth connector it also resolves the stored +/// refresh token into the credential the server reads and injects it into the +/// process env per the delivery spec (§15). Non-OAuth rows are returned unchanged. +/// +/// `registry` is the system pool, where `oauth_providers` (the client credentials) +/// lives. A resolution failure is logged, not fatal: the server still starts, and +/// fails its own auth visibly, rather than the whole login batch aborting. +pub async fn user_row_spec_resolved( + row: &crate::db::mcp_user_servers::McpUserServerRow, + container: &str, + registry: &SqlitePool, +) -> McpServerSpec { + let mut spec = user_row_spec(row, container); + if let (Some(provider), Some(deliver), Some(refresh)) = + (row.oauth_provider.as_deref(), row.deliver(), row.api_key.as_deref()) + { + if let Err(e) = inject_oauth_env(&mut spec, provider, &deliver, refresh, registry).await { + warn!("connector '{}': OAuth credential delivery failed: {e}", row.name); + } + } + spec +} + +/// Assembles the credential from the provider's client creds + the refresh token and +/// sets it on `spec.config.env` under the delivery spec's env name. +async fn inject_oauth_env( + spec: &mut McpServerSpec, + provider_name: &str, + deliver: &DeliverSpec, + refresh_token: &str, + registry: &SqlitePool, +) -> Result<()> { + if deliver.as_ != "env" { + anyhow::bail!("only `env` credential delivery is wired (deliver.as = `{}`)", deliver.as_); + } + let env_name = deliver.env.as_deref() + .ok_or_else(|| anyhow::anyhow!("deliver.as=env but no deliver.env name"))?; + let format = deliver.format.as_deref().unwrap_or("google_authorized_user"); + let provider = crate::db::oauth_providers::get(registry, provider_name).await? + .ok_or_else(|| anyhow::anyhow!("unknown OAuth provider `{provider_name}`"))?; + let cred = oauth::assemble_credential(format, &provider, refresh_token)?; + spec.config.env.get_or_insert_with(HashMap::new).insert(env_name.to_string(), cred); + Ok(()) +} + /// Generates a 32-char alphanumeric id for a persisted media filename /// (mirrors `ImageGeneratorManager`). fn random_id() -> String { @@ -510,3 +586,50 @@ pub fn content_type_for_ext(ext: &str) -> &'static str { _ => "application/octet-stream", } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn key_placeholder_legacy_is_substituted() { + let (url, key) = apply_key_placeholder( + Some("https://x/?k={key}".into()), + Some("secret123".into()), + ); + assert_eq!(url.as_deref(), Some("https://x/?k=secret123")); + assert!(key.is_none(), "api_key is consumed after substitution"); + } + + #[test] + fn key_placeholder_secret_token_is_substituted() { + // Tavily's unified form: the URL carries {SECRET:tavilyApiKey}. + let (url, key) = apply_key_placeholder( + Some("https://mcp.tavily.com/mcp/?tavilyApiKey={SECRET:tavilyApiKey}".into()), + Some("tvly-abc".into()), + ); + assert_eq!(url.as_deref(), Some("https://mcp.tavily.com/mcp/?tavilyApiKey=tvly-abc")); + assert!(key.is_none(), "api_key is consumed when the URL had a SECRET token"); + } + + #[test] + fn key_placeholder_no_token_keeps_key_for_bearer() { + // No placeholder in the URL → the key stays, so the HTTP transport + // sends it as `Authorization: Bearer`. + let (url, key) = apply_key_placeholder( + Some("https://x.example.com/mcp".into()), + Some("bearer-key".into()), + ); + assert_eq!(url.as_deref(), Some("https://x.example.com/mcp")); + assert_eq!(key.as_deref(), Some("bearer-key")); + } + + #[test] + fn substitute_secret_tokens_replaces_every_occurrence() { + let s = substitute_secret_tokens( + "a={SECRET:K}&b={SECRET:K}&c={ENV:C}", + "VAL", + ); + assert_eq!(s, "a=VAL&b=VAL&c={ENV:C}"); + } +} diff --git a/crates/skald-core/src/mcp/oauth.rs b/crates/skald-core/src/mcp/oauth.rs new file mode 100644 index 0000000..9c108f1 --- /dev/null +++ b/crates/skald-core/src/mcp/oauth.rs @@ -0,0 +1,200 @@ +//! OAuth 2.0 authorization-code + PKCE for per-user connectors (blueprint §15). +//! +//! The consent step is a **human copy-paste**, not a headless action (§15): Skald +//! builds a consent URL, the user approves it in a browser, and the provider lands +//! the `code` on a static page (`redirect_uri`, e.g. `oauth/show.html`) that shows +//! it for copying. Skald then exchanges the code for a refresh token. PKCE means an +//! intercepted code is useless without the verifier, which never leaves this +//! process — so the copy-paste page can be a plain static file with no backend. +//! +//! The obtained refresh token is delivered to the connector's server per its +//! manifest `auth.deliver` spec; only `env` delivery is wired (the credential is +//! injected as an environment variable at `docker exec` time — nothing on disk). + +use anyhow::{Context, Result, bail}; +use base64::Engine; +use rand::Rng as _; +use serde::Deserialize; +use sha2::{Digest, Sha256}; + +use crate::db::oauth_providers::OauthProviderRow; + +/// How Skald delivers the obtained credential to the connector's server process, +/// mirrored from the manifest's `auth.deliver` (§15). +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct DeliverSpec { + /// `env` | `file`. Only `env` is implemented; a `file` target is rejected at + /// activation with a clear message rather than silently half-working. + #[serde(rename = "as")] + pub as_: String, + /// The serialization Skald must produce (`google_authorized_user` | `refresh_token`). + #[serde(default)] + pub format: Option, + /// `as=env`: the environment variable the credential is injected into. + #[serde(default)] + pub env: Option, + /// `as=file`: the target path (unused while file delivery is unimplemented). + #[serde(default)] + pub path: Option, +} + +const URL_SAFE: base64::engine::general_purpose::GeneralPurpose = + base64::engine::general_purpose::URL_SAFE_NO_PAD; + +/// A high-entropy PKCE verifier and its S256 challenge (RFC 7636). +pub struct Pkce { + pub verifier: String, + pub challenge: String, +} + +pub fn generate_pkce() -> Pkce { + let mut bytes = [0u8; 32]; + rand::rng().fill_bytes(&mut bytes); + let verifier = URL_SAFE.encode(bytes); // 43-char base64url, within the RFC range + let challenge = URL_SAFE.encode(Sha256::digest(verifier.as_bytes())); + Pkce { verifier, challenge } +} + +/// An opaque, URL-safe `state` value: CSRF guard and the key of the pending flow. +pub fn random_state() -> String { + let mut bytes = [0u8; 24]; + rand::rng().fill_bytes(&mut bytes); + URL_SAFE.encode(bytes) +} + +/// Builds the authorization-endpoint URL the user opens to consent. Merges the +/// provider's `extra_params` (Google needs `access_type=offline` + `prompt=consent` +/// to return a refresh token) after the standard params. +pub fn build_consent_url( + provider: &OauthProviderRow, + scopes: &[String], + state: &str, + challenge: &str, +) -> Result { + let scope = scopes.join(" "); + let mut params: Vec<(String, String)> = vec![ + ("client_id".into(), provider.client_id.clone()), + ("redirect_uri".into(), provider.redirect_uri.clone()), + ("response_type".into(), "code".into()), + ("scope".into(), scope), + ("state".into(), state.into()), + ("code_challenge".into(), challenge.into()), + ("code_challenge_method".into(), "S256".into()), + ]; + for (k, v) in provider.extra() { + params.push((k, v)); + } + let url = reqwest::Url::parse_with_params(&provider.auth_url, ¶ms) + .with_context(|| format!("invalid authorization endpoint `{}`", provider.auth_url))?; + Ok(url.to_string()) +} + +/// The token endpoint's response. Google returns `refresh_token` only on the first +/// consent for a client, or when `prompt=consent` forces re-issue — hence the +/// provider's `extra_params`. +#[derive(Debug, Deserialize)] +pub struct TokenResponse { + #[serde(default)] pub access_token: Option, + #[serde(default)] pub refresh_token: Option, + #[serde(default)] pub expires_in: Option, + #[serde(default)] pub scope: Option, + #[serde(default)] pub error: Option, + #[serde(default)] pub error_description: Option, +} + +/// Exchanges an authorization `code` (+ PKCE `verifier`) for tokens at the +/// provider's token endpoint. +pub async fn exchange_code( + provider: &OauthProviderRow, + code: &str, + verifier: &str, +) -> Result { + let params = [ + ("grant_type", "authorization_code"), + ("code", code), + ("client_id", provider.client_id.as_str()), + ("client_secret", provider.client_secret.as_str()), + ("redirect_uri", provider.redirect_uri.as_str()), + ("code_verifier", verifier), + ]; + // `RequestBuilder::form` needs reqwest's `urlencoded` feature, which this build + // doesn't enable — so encode the body ourselves. Parsing a throwaway URL with + // these params yields exactly the `application/x-www-form-urlencoded` string. + let body = reqwest::Url::parse_with_params("http://form.local/", ¶ms) + .ok() + .and_then(|u| u.query().map(str::to_owned)) + .unwrap_or_default(); + let resp = reqwest::Client::new() + .post(&provider.token_url) + .header(reqwest::header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .body(body) + .send() + .await + .context("token endpoint request failed")?; + let status = resp.status(); + let body: TokenResponse = resp + .json() + .await + .context("token endpoint returned a non-JSON body")?; + if let Some(err) = &body.error { + let detail = body.error_description.as_deref() + .map(|d| format!(" — {d}")).unwrap_or_default(); + bail!("token exchange failed: {err}{detail}"); + } + if !status.is_success() { + bail!("token exchange failed with HTTP {status}"); + } + Ok(body) +} + +/// Serializes a refresh token into the shape the connector's server reads, per +/// `deliver.format`. `google_authorized_user` is the JSON that +/// `google.oauth2.credentials.Credentials.from_authorized_user_info` accepts — the +/// server refreshes access tokens from it on its own. +pub fn assemble_credential( + format: &str, + provider: &OauthProviderRow, + refresh_token: &str, +) -> Result { + match format { + "google_authorized_user" => Ok(serde_json::json!({ + "type": "authorized_user", + "client_id": provider.client_id, + "client_secret": provider.client_secret, + "refresh_token": refresh_token, + "token_uri": provider.token_url, + }).to_string()), + "refresh_token" => Ok(refresh_token.to_string()), + other => bail!("unsupported deliver.format `{other}`"), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn pkce_challenge_is_s256_of_verifier() { + let p = generate_pkce(); + let expect = URL_SAFE.encode(Sha256::digest(p.verifier.as_bytes())); + assert_eq!(p.challenge, expect); + assert!(!p.verifier.contains(['+', '/', '=']), "verifier must be url-safe, unpadded"); + } + + #[test] + fn authorized_user_credential_has_googles_fields() { + let provider = OauthProviderRow { + name: "google".into(), display_name: "Google".into(), + auth_url: "https://a".into(), token_url: "https://t".into(), + client_id: "cid".into(), client_secret: "csec".into(), + redirect_uri: "https://r".into(), extra_params: None, + created_at: String::new(), updated_at: String::new(), + }; + let cred = assemble_credential("google_authorized_user", &provider, "rt-123").unwrap(); + let v: serde_json::Value = serde_json::from_str(&cred).unwrap(); + assert_eq!(v["type"], "authorized_user"); + assert_eq!(v["client_id"], "cid"); + assert_eq!(v["refresh_token"], "rt-123"); + assert_eq!(v["token_uri"], "https://t"); + } +} diff --git a/crates/skald-core/src/mcp/verify.rs b/crates/skald-core/src/mcp/verify.rs new file mode 100644 index 0000000..1b59d82 --- /dev/null +++ b/crates/skald-core/src/mcp/verify.rs @@ -0,0 +1,417 @@ +//! Verify-before-save for MCP connectors (blueprint §15 verify step). +//! +//! When a user fills the activation form, Skald can run the connector's declared +//! `verify` command to confirm the credentials actually work *before* persisting +//! the activation. This module owns: +//! +//! - [`apply_placeholders`] — the single substitution engine for `{ENV:NAME}` +//! and `{SECRET:NAME}` tokens (used here for the verify command, and by the +//! MCP transport for URLs / env values). +//! - [`run_verify`] — launches the resolved command either on the host (for a +//! global `mcp_remote` connector) or inside the caller's container (for a +//! per-user `mcp_local` connector), parses the JSON result, and returns a +//! [`VerifyReport`]. +//! +//! Output contract: the verify command must print one JSON object on stdout, +//! `{"ok": bool, "message": string, "details"?: object}`, and exit 0 on success. +//! If the JSON parse fails, [`run_verify`] falls back to the exit code. Secrets +//! are never logged. + +use std::collections::HashMap; +use std::path::Path; +use std::process::Stdio; +use std::time::{Duration, Instant}; + +use serde::Serialize; +use serde_json::Value; +use tokio::io::AsyncReadExt; +use tracing::debug; + +/// Default timeout for a verify command (seconds). Overridable per-connector via +/// the manifest's `verify.timeout_secs`. +pub const DEFAULT_VERIFY_TIMEOUT_SECS: u64 = 15; + +/// The outcome of a verify run, surfaced to the UI verbatim. +#[derive(Debug, Clone, Serialize)] +pub struct VerifyReport { + /// `true` when the credentials check out. + pub ok: bool, + /// Human-readable result line (shown next to the Test button). + pub message: String, + /// Optional structured details (shown in a `
` block). Never holds
+    /// secrets — the verify script is responsible for not echoing them.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub details: Option,
+    /// Wall-clock time the command took.
+    #[serde(skip)]
+    pub elapsed: Duration,
+    /// `true` when the connector declares no `verify` step, so "no test" must
+    /// be distinguishable from "test passed" in the UI.
+    #[serde(skip)]
+    pub skipped: bool,
+}
+
+impl VerifyReport {
+    /// Synthesized when the connector has no `verify` step — the UI shows
+    /// "no test available" rather than a pass/fail.
+    pub fn skipped() -> Self {
+        Self {
+            ok: true,
+            message: "This connector has no verification step.".into(),
+            details: None,
+            elapsed: Duration::ZERO,
+            skipped: true,
+        }
+    }
+}
+
+/// Where [`run_verify`] executes the command. Mirrors `McpServerSpec.launch_in`:
+/// `None` runs on the host (a global `mcp_remote` connector), `Some(container)`
+/// runs inside the user's container via `docker exec`.
+pub enum VerifyTarget<'a> {
+    /// Run on the Skald host process. `workdir` is an absolute host path
+    /// (typically `/scripts//`).
+    Host { workdir: &'a Path },
+    /// Run inside the user's sandbox container. `workdir` is an absolute path
+    /// *inside* the container (e.g. `/root/.skald/mcp/`).
+    Container {
+        container: &'a str,
+        workdir: &'a Path,
+    },
+}
+
+/// Substitutes `{ENV:NAME}` and `{SECRET:NAME}` tokens in `text`.
+///
+/// - `{ENV:NAME}` → `env[NAME]`, or empty string if absent.
+/// - `{SECRET:NAME}` → `secret[NAME]`, or empty string if absent.
+/// - Any other `{...}` token is left untouched — `{key}` belongs to the remote
+///   transport's URL substitution (see `mcp::apply_key`), and anything else is a
+///   misconfiguration that should stay visible rather than be silently erased.
+pub fn apply_placeholders(
+    text: &str,
+    env: &HashMap,
+    secret: &HashMap,
+) -> String {
+    let mut out = String::with_capacity(text.len());
+    let mut rest = text;
+    while let Some(open) = rest.find('{') {
+        // Append everything up to the '{'.
+        out.push_str(&rest[..open]);
+        let after = &rest[open..];
+        if let Some(close) = after.find('}') {
+            let token = &after[..=close]; // includes both braces
+            let inner = token.strip_prefix('{').unwrap().strip_suffix('}').unwrap();
+            // ENV:/SECRET: tokens are always consumed (missing key → empty);
+            // any other `{...}` is left untouched so a misconfiguration stays
+            // visible rather than being silently erased.
+            if let Some(name) = inner.strip_prefix("ENV:") {
+                out.push_str(env.get(name).map(|s| s.as_str()).unwrap_or(""));
+            } else if let Some(name) = inner.strip_prefix("SECRET:") {
+                out.push_str(secret.get(name).map(|s| s.as_str()).unwrap_or(""));
+            } else {
+                out.push_str(token);
+            }
+            rest = &after[close + 1..];
+        } else {
+            // No closing brace — emit the rest literally and stop.
+            out.push_str(after);
+            return out;
+        }
+    }
+    out.push_str(rest);
+    out
+}
+
+/// Runs the verify `command` (after placeholder substitution) in the given
+/// target, injects the env/secret values as environment variables, captures
+/// stdout/stderr under a timeout, and parses the JSON result.
+///
+/// The command and resolved env are NOT logged (secrets may be inline). Only
+/// the final `ok`/`message` are traced at debug level.
+pub async fn run_verify(
+    command: &str,
+    env_values: &HashMap,
+    secret_values: &HashMap,
+    target: VerifyTarget<'_>,
+    timeout_secs: u64,
+) -> VerifyReport {
+    let resolved = apply_placeholders(command, env_values, secret_values);
+    let timeout = Duration::from_secs(timeout_secs.max(1));
+    let started = Instant::now();
+
+    // Build the process: `docker exec … sh -c ""` or host `sh -c ""`.
+    let mut cmd = match target {
+        VerifyTarget::Container { container, workdir } => {
+            let mut c = tokio::process::Command::new("docker");
+            c.arg("exec")
+                .arg("-w").arg(workdir)
+                .arg(container);
+            inject_env_flags(&mut c, env_values, secret_values);
+            c.arg("sh").arg("-c").arg(&resolved);
+            c
+        }
+        VerifyTarget::Host { workdir } => {
+            let mut c = tokio::process::Command::new("sh");
+            c.arg("-c").arg(&resolved).current_dir(workdir);
+            inject_env_vars(&mut c, env_values, secret_values);
+            c
+        }
+    };
+    cmd.stdout(Stdio::piped())
+        .stderr(Stdio::piped())
+        .stdin(Stdio::null())
+        .kill_on_drop(true);
+
+    let child = match cmd.spawn() {
+        Ok(c) => c,
+        Err(e) => {
+            return VerifyReport {
+                ok: false,
+                message: format!("Could not start the verify command: {e}"),
+                details: None,
+                elapsed: started.elapsed(),
+                skipped: false,
+            };
+        }
+    };
+
+    let outcome = run_with_timeout(child, timeout).await;
+    let elapsed = started.elapsed();
+
+    let report = parse_verify_output(&outcome, elapsed);
+    debug!(ok = report.ok, elapsed_ms = elapsed.as_millis() as u64, "verify");
+    report
+}
+
+/// Collects the child's stdout/stderr under a single timeout, returning the
+/// captured buffers and the exit code (None if killed by timeout).
+async fn run_with_timeout(
+    mut child: tokio::process::Child,
+    timeout: Duration,
+) -> VerifyOutcome {
+    let mut stdout = child.stdout.take().expect("stdout piped");
+    let mut stderr = child.stderr.take().expect("stderr piped");
+
+    let collect = async {
+        let mut out = Vec::new();
+        let mut err = Vec::new();
+        // Read concurrently — the pipes are independent.
+        let r1 = stdout.read_to_end(&mut out);
+        let r2 = stderr.read_to_end(&mut err);
+        let (ro, re, status) = tokio::join!(r1, r2, child.wait());
+        ro.map_err(anyhow::Error::from)?;
+        re.map_err(anyhow::Error::from)?;
+        let code = status.ok().and_then(|s| s.code());
+        Ok::<_, anyhow::Error>((out, err, code))
+    };
+
+    match tokio::time::timeout(timeout, collect).await {
+        Ok(Ok((out, err, code))) => VerifyOutcome { stdout: out, stderr: err, code, timed_out: false },
+        // Inner error (spawn/io).
+        Ok(Err(e)) => VerifyOutcome {
+            stdout: Vec::new(),
+            stderr: e.to_string().into_bytes(),
+            code: None,
+            timed_out: false,
+        },
+        // Timeout: kill_on_drop takes care of the child.
+        Err(_) => VerifyOutcome {
+            stdout: Vec::new(),
+            stderr: format!("verify timed out after {}s", timeout.as_secs()).into_bytes(),
+            code: None,
+            timed_out: true,
+        },
+    }
+}
+
+struct VerifyOutcome {
+    stdout: Vec,
+    stderr: Vec,
+    code: Option,
+    timed_out: bool,
+}
+
+/// Parses the verify command's output into a [`VerifyReport`].
+///
+/// Contract: the command prints one JSON object on stdout:
+/// `{"ok": bool, "message": string, "details"?: object}`. If the parse fails,
+/// falls back to the exit code (0 = ok, anything else = fail) and uses stderr
+/// (or stdout) as the message.
+fn parse_verify_output(outcome: &VerifyOutcome, elapsed: Duration) -> VerifyReport {
+    let stdout = String::from_utf8_lossy(&outcome.stdout);
+    let stderr = String::from_utf8_lossy(&outcome.stderr);
+
+    if outcome.timed_out {
+        return VerifyReport {
+            ok: false,
+            message: stderr.trim().to_string(),
+            details: None,
+            elapsed,
+            skipped: false,
+        };
+    }
+
+    // Try JSON parse first (prefer the last line, in case the script emitted a
+    // trailing newline or a preamble).
+    let trimmed = stdout.trim();
+    if !trimmed.is_empty() {
+        if let Ok(v) = serde_json::from_str::(trimmed) {
+            let ok = v.get("ok").and_then(|o| o.as_bool()).unwrap_or_else(|| outcome.code == Some(0));
+            let message = v
+                .get("message")
+                .and_then(|m| m.as_str())
+                .unwrap_or("")
+                .to_string();
+            let details = v.get("details").cloned();
+            return VerifyReport { ok, message, details, elapsed, skipped: false };
+        }
+    }
+
+    // Fallback: exit-code semantics. Empty stdout → fall back to stderr.
+    let ok = outcome.code == Some(0);
+    let message = if !trimmed.is_empty() {
+        trimmed.to_string()
+    } else if !stderr.trim().is_empty() {
+        stderr.trim().to_string()
+    } else if ok {
+        "Verification succeeded.".into()
+    } else {
+        format!("Verify failed (exit code {}).", outcome.code.unwrap_or(-1))
+    };
+    VerifyReport { ok, message, details: None, elapsed, skipped: false }
+}
+
+/// Adds `-e KEY=VALUE` flags for `docker exec`, for both env and secret values.
+fn inject_env_flags(
+    cmd: &mut tokio::process::Command,
+    env: &HashMap,
+    secret: &HashMap,
+) {
+    for (k, v) in env.iter().chain(secret.iter()) {
+        cmd.arg("-e").arg(format!("{k}={v}"));
+    }
+}
+
+/// Sets environment variables for a host `sh -c` process.
+fn inject_env_vars(
+    cmd: &mut tokio::process::Command,
+    env: &HashMap,
+    secret: &HashMap,
+) {
+    for (k, v) in env.iter().chain(secret.iter()) {
+        cmd.env(k, v);
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn m(items: &[(&str, &str)]) -> HashMap {
+        items.iter().map(|(k, v)| (k.to_string(), v.to_string())).collect()
+    }
+
+    #[test]
+    fn placeholders_env_and_secret() {
+        let env = m(&[("HOST", "imap.example.com"), ("PORT", "993")]);
+        let secret = m(&[("PASS", "hunter2")]);
+        let s = apply_placeholders("h={ENV:HOST} p={ENV:PORT} s={SECRET:PASS}", &env, &secret);
+        assert_eq!(s, "h=imap.example.com p=993 s=hunter2");
+    }
+
+    #[test]
+    fn placeholders_missing_become_empty() {
+        let env = m(&[("HOST", "x")]);
+        let secret = HashMap::new();
+        let s = apply_placeholders("[{ENV:HOST}][{ENV:MISSING}][{SECRET:X}]", &env, &secret);
+        assert_eq!(s, "[x][][]");
+    }
+
+    #[test]
+    fn placeholders_unknown_left_untouched() {
+        let env = HashMap::new();
+        let secret = HashMap::new();
+        let s = apply_placeholders("{key} {ENV:A} {0}", &env, &secret);
+        assert_eq!(s, "{key}  {0}");
+    }
+
+    #[test]
+    fn placeholders_no_braces() {
+        let env = HashMap::new();
+        let secret = HashMap::new();
+        assert_eq!(apply_placeholders("plain text", &env, &secret), "plain text");
+    }
+
+    #[test]
+    fn placeholders_unclosed_brace_kept() {
+        let env = HashMap::new();
+        let secret = HashMap::new();
+        assert_eq!(apply_placeholders("a {ENV:B c", &env, &secret), "a {ENV:B c");
+    }
+
+    #[test]
+    fn parse_json_ok() {
+        let o = VerifyOutcome {
+            stdout: br#"{"ok": true, "message": "all good"}"#.to_vec(),
+            stderr: vec![],
+            code: Some(0),
+            timed_out: false,
+        };
+        let r = parse_verify_output(&o, Duration::from_millis(10));
+        assert!(r.ok);
+        assert_eq!(r.message, "all good");
+    }
+
+    #[test]
+    fn parse_json_fail_with_details() {
+        let o = VerifyOutcome {
+            stdout: br#"{"ok": false, "message": "bad creds", "details": {"imap": "ok", "smtp": "no"}}"#.to_vec(),
+            stderr: vec![],
+            code: Some(1),
+            timed_out: false,
+        };
+        let r = parse_verify_output(&o, Duration::from_millis(10));
+        assert!(!r.ok);
+        assert_eq!(r.message, "bad creds");
+        assert_eq!(r.details.unwrap()["smtp"], "no");
+    }
+
+    #[test]
+    fn parse_fallback_exit_code() {
+        let o = VerifyOutcome {
+            stdout: b"some plain output".to_vec(),
+            stderr: vec![],
+            code: Some(0),
+            timed_out: false,
+        };
+        let r = parse_verify_output(&o, Duration::from_millis(10));
+        assert!(r.ok);
+        assert_eq!(r.message, "some plain output");
+    }
+
+    #[test]
+    fn parse_fallback_stderr_on_fail() {
+        let o = VerifyOutcome {
+            stdout: vec![],
+            stderr: b"connection refused".to_vec(),
+            code: Some(2),
+            timed_out: false,
+        };
+        let r = parse_verify_output(&o, Duration::from_millis(10));
+        assert!(!r.ok);
+        assert_eq!(r.message, "connection refused");
+    }
+
+    #[test]
+    fn parse_timeout_is_fail() {
+        let o = VerifyOutcome {
+            stdout: vec![],
+            stderr: b"verify timed out after 15s".to_vec(),
+            code: None,
+            timed_out: true,
+        };
+        let r = parse_verify_output(&o, Duration::from_secs(15));
+        assert!(!r.ok);
+        assert!(r.message.contains("timed out"));
+    }
+}
diff --git a/crates/skald-core/src/skald/user_context.rs b/crates/skald-core/src/skald/user_context.rs
index b15f1de..9476ad5 100644
--- a/crates/skald-core/src/skald/user_context.rs
+++ b/crates/skald-core/src/skald/user_context.rs
@@ -200,14 +200,18 @@ impl UserContextFactory {
         {
             let um        = Arc::clone(&user_mcp);
             let upool     = Arc::clone(&pool);
+            let registry  = Arc::clone(&self.registry_pool);
             let container = crate::container::container_name(user_id);
             let mname: &'static str = Box::leak(format!("mcp:{user_id}").into_boxed_str());
             self.supervisor.adopt_one(mname, tokio::spawn(async move {
                 match crate::db::mcp_user_servers::all_startable(&upool).await {
                     Ok(rows) => {
-                        let specs = rows.iter()
-                            .map(|r| crate::mcp::user_row_spec(r, &container))
-                            .collect();
+                        let mut specs = Vec::with_capacity(rows.len());
+                        for r in &rows {
+                            // OAuth connectors resolve their stored refresh token into
+                            // the env-delivered credential here (§15).
+                            specs.push(crate::mcp::user_row_spec_resolved(r, &container, ®istry).await);
+                        }
                         um.connect_all(specs, false).await;
                     }
                     Err(e) => tracing::warn!(error = %e, "per-user MCP init: failed to read mcp_user_servers"),
diff --git a/crates/skald-core/src/tools/fs/grep_files.rs b/crates/skald-core/src/tools/fs/grep_files.rs
index 9b5af49..7461847 100644
--- a/crates/skald-core/src/tools/fs/grep_files.rs
+++ b/crates/skald-core/src/tools/fs/grep_files.rs
@@ -173,9 +173,8 @@ impl Tool for GrepFiles {
     }
 }
 
-// `secrets` is skipped so a recursive grep rooted at a parent (e.g. the auto-read
-// working directory) never descends into and leaks secret values.
-const SKIP_DIRS: &[&str] = &["target", ".git", "node_modules", ".venv", "__pycache__", "secrets"];
+// Noise, not policy: build output and vendored trees a grep is never looking for.
+const SKIP_DIRS: &[&str] = &["target", ".git", "node_modules", ".venv", "__pycache__"];
 const MAX_FILE_BYTES: u64 = 200_000;
 const MAX_OUTPUT_BYTES: usize = 60_000;
 const MAX_LINE_BYTES: usize = 500;
diff --git a/crates/skald-core/src/tools/fs/list_files.rs b/crates/skald-core/src/tools/fs/list_files.rs
index 373c4e7..5a60529 100644
--- a/crates/skald-core/src/tools/fs/list_files.rs
+++ b/crates/skald-core/src/tools/fs/list_files.rs
@@ -11,10 +11,8 @@ use crate::tools::{
 };
 use super::{classify_memory, resolve, MemScope};
 
-/// Directories to skip unconditionally when walking.
-/// `secrets` is skipped so a recursive listing rooted at a parent (e.g. the auto-read
-/// working directory) never reveals the contents of the secrets store.
-const SKIP_DIRS: &[&str] = &[".git", "target", "node_modules", ".cache", "secrets"];
+/// Directories to skip unconditionally when walking — noise, not policy.
+const SKIP_DIRS: &[&str] = &[".git", "target", "node_modules", ".cache"];
 
 pub struct ListFiles {
     /// The `shared-memory` (system) pool; see [`ReadFile`](super::ReadFile).
diff --git a/crates/skald-core/src/tools/fs/mod.rs b/crates/skald-core/src/tools/fs/mod.rs
index fab9638..92c2d52 100644
--- a/crates/skald-core/src/tools/fs/mod.rs
+++ b/crates/skald-core/src/tools/fs/mod.rs
@@ -110,7 +110,7 @@ pub fn resolve(user_path: &str) -> Result {
 /// does not exist yet) is appended lexically. Falls back to a pure lexical normalization
 /// when nothing along the path can be canonicalized.
 ///
-/// This closes `docs/../secrets/x` traversal and symlink escapes for both the allow
+/// This closes `docs/../private/x` traversal and symlink escapes for both the allow
 /// fast-paths (`RunContext`) and the deny rules (`approval::normalize_path`).
 pub fn canonicalize_for_policy(path: &str, base: &Path) -> PathBuf {
     let raw = {
diff --git a/src/frontend/api/marketplace.rs b/src/frontend/api/marketplace.rs
index ab49c54..fc26779 100644
--- a/src/frontend/api/marketplace.rs
+++ b/src/frontend/api/marketplace.rs
@@ -129,6 +129,14 @@ struct AuthSpec {
     #[serde(default, rename = "type")] kind: Option,
     #[serde(default)] delivery: Option,
     #[serde(default)] scopes:   Vec,
+    /// oauth: the identity provider slug (`google`) — resolved to client creds +
+    /// endpoints from the `oauth_providers` registry table (§15). Never carries
+    /// URLs or secrets: those stay out of the public feed by design.
+    #[serde(default)] provider:  Option,
+    /// oauth: how the obtained credential is delivered to the server process
+    /// (`{as,format,env,path}`). Parsed with skald-core's own type so the stored
+    /// snapshot and the runtime injector agree on the shape.
+    #[serde(default)] deliver:   Option,
 }
 
 #[derive(Debug, Clone, Default, Deserialize)]
@@ -150,6 +158,27 @@ struct McpConfigManifest {
     #[serde(default)] transport: Option,
 }
 
+/// One env/secret field the activation UI must collect (the feed's `env[]` entry).
+/// Richer than the old `Vec` of bare key names — drives a real form.
+#[derive(Debug, Clone, Default, Serialize, Deserialize)]
+struct EnvEntry {
+    name:        String,
+    label:       String,
+    description: String,
+    #[serde(default)] required: bool,
+    #[serde(default)] secret:   bool,
+    #[serde(default)] default:  Option,
+    #[serde(default)] example:  Option,
+}
+
+/// The verify-before-save step. `command` is a shell snippet run with the
+/// collected env/secret injected; `timeout_secs` defaults to 15.
+#[derive(Debug, Clone, Default, Deserialize)]
+struct VerifySpec {
+    command:      String,
+    #[serde(default)] timeout_secs: Option,
+}
+
 #[derive(Debug, Clone, Default, Deserialize)]
 struct Manifest {
     #[serde(default)] name:               Option,
@@ -167,12 +196,19 @@ struct Manifest {
     #[serde(default)] files:              Vec,
     #[serde(default)] scope:              Option,
     #[serde(default)] auth:               Option,
+    /// The full env/secret schema for the activation form (objects, not key names).
+    #[serde(default)] env:                Vec,
+    /// Optional verify-before-save command.
+    #[serde(default)] verify:             Option,
 }
 
 #[derive(Debug, Clone)]
 struct Hydrated {
     entry:    IndexEntry,
     manifest: Manifest,
+    /// The manifest exactly as served, so [`install`] can record it verbatim
+    /// without a second fetch. `None` when the manifest could not be read.
+    manifest_raw: Option,
 }
 
 // ── feed vocabulary → Skald vocabulary ────────────────────────────────────────
@@ -373,12 +409,17 @@ async fn fetch_feed() -> Result, ApiError> {
         set.spawn(async move {
             let url = format!("{}/{}/connector.json", base, folder_of(&entry));
             // A manifest that fails to load degrades that one card to whatever the
-            // index said; it never fails the whole listing.
-            let manifest = match HTTP.get(&url).send().await {
-                Ok(r) => r.json::().await.unwrap_or_default(),
-                Err(_) => Manifest::default(),
+            // index said; it never fails the whole listing. The raw text is kept
+            // alongside the parsed form so `install` can record what was served
+            // even if some field of it did not parse.
+            let (manifest, manifest_raw) = match HTTP.get(&url).send().await {
+                Ok(r) => match r.text().await {
+                    Ok(t)  => (serde_json::from_str::(&t).unwrap_or_default(), Some(t)),
+                    Err(_) => (Manifest::default(), None),
+                },
+                Err(_) => (Manifest::default(), None),
             };
-            Hydrated { entry, manifest }
+            Hydrated { entry, manifest, manifest_raw }
         });
     }
 
@@ -548,10 +589,10 @@ pub struct InstallBody {
 }
 
 /// Imports a feed entry into `mcp_catalog` — the act that moves a connector from
-/// "someone else vetted this" to "this household's admin accepted it". For an
-/// `mcp_local` entry it first downloads and hash-verifies the scripts into
-/// `./scripts//`, which is code landing on the box and therefore needs
-/// `mcp.register_local_script` (§14) on top of `mcp.manage_catalog`.
+/// "someone else vetted this" to "this household's admin accepted it". It downloads
+/// and hash-verifies the connector's folder into `./connectors//`; for an
+/// `mcp_local` entry that folder holds code which will run on this box, and
+/// therefore needs `mcp.register_local_script` (§14) on top of `mcp.manage_catalog`.
 ///
 /// Installing does **not** activate: a global entry still needs the admin to
 /// enable it with a key, a per-user one still needs each user to activate it.
@@ -579,9 +620,25 @@ pub async fn install(
     }
 
     // Download + verify before touching the catalog, so a failed digest leaves no
-    // trace of a half-installed connector.
-    let (script_path, verified) = if source == "local_script" {
-        let files = download_verified(&h.entry, &h.manifest).await?;
+    // trace of a half-installed connector. A `local_script` always downloads its
+    // files; a `remote` connector downloads them only if it declares a `verify`
+    // step that references a script (otherwise there is nothing to fetch).
+    let verify_command = h.manifest.verify.as_ref().map(|v| v.command.clone());
+    let verify_timeout = h.manifest.verify.as_ref().and_then(|v| v.timeout_secs);
+    let all_files = files_of(&h.entry, &h.manifest);
+    let verify_script_rel = verify_command
+        .as_deref()
+        .and_then(|c| verify_script_of(c, &all_files));
+    let verify_script_path = verify_script_rel
+        .as_ref()
+        .map(|f| format!("{}/{}", body.id, f));
+
+    // Every source downloads its folder now — a remote connector has an icon and a
+    // manifest to record even when it has no code to run here.
+    let installed =
+        download_verified(&h.entry, &h.manifest, h.manifest_raw.as_deref(), &source).await?;
+
+    let script_path = if source == "local_script" {
         let entry_file = cfg
             .args
             .first()
@@ -590,9 +647,9 @@ pub async fn install(
                 "manifest has no mcp_config.args[0] naming the script to run",
             ))?;
         let entry_file = safe_rel_path(&entry_file)?.to_string();
-        (Some(format!("{}/{}", body.id, entry_file)), files)
+        Some(format!("{}/{}", body.id, entry_file))
     } else {
-        (None, 0)
+        None
     };
 
     let args_json = if source == "local_script" {
@@ -605,9 +662,23 @@ pub async fn install(
         serde_json::to_string(&cfg.args).ok()
     };
 
-    // `requires` is the feed's coarse precondition list; the activation UI needs
-    // the concrete env keys, which only the manifest's mcp_config knows.
-    let config_schema: Vec = cfg.env.keys().cloned().collect();
+    // The full env/secret schema for the activation form. The manifest's top-level
+    // `env[]` (array of objects with label/description/required/secret/…) is the
+    // source of truth; if a feed only ships the old `mcp_config.env` placeholder
+    // map, fall back to bare key names so the form still renders.
+    let config_schema_json = if !h.manifest.env.is_empty() {
+        serde_json::to_string(&h.manifest.env).ok()
+    } else if !cfg.env.is_empty() {
+        let names: Vec = cfg.env.keys().cloned().collect();
+        serde_json::to_string(&names).ok()
+    } else {
+        None
+    };
+    let _ = verify_timeout; // surfaced to the runtime via the verify module's default
+
+    let folder = folder_of(&h.entry);
+    let icon_small_path = installed_icon(h.entry.icon_small.as_deref(), &folder, &installed);
+    let icon_large_path = installed_icon(h.entry.icon_large.as_deref(), &folder, &installed);
 
     let id = mcp_catalog::upsert(
         skald.db(),
@@ -621,9 +692,23 @@ pub async fn install(
             env_json:           if cfg.env.is_empty() { None } else { serde_json::to_string(&cfg.env).ok() },
             url:                cfg.url.as_deref(),
             script_path:        script_path.as_deref(),
-            config_schema_json: if config_schema.is_empty() { None } else { serde_json::to_string(&config_schema).ok() },
+            config_schema_json,
             auth_kind:          &norm_auth_kind(&h.entry, &h.manifest),
+            // OAuth wiring (§15): provider slug, the scopes shown at consent, and the
+            // credential delivery spec — all snapshotted so an activation is
+            // reproducible even if the feed later changes.
+            oauth_provider:     h.manifest.auth.as_ref().and_then(|a| a.provider.as_deref()),
+            oauth_scopes_json:  h.manifest.auth.as_ref()
+                                    .filter(|a| !a.scopes.is_empty())
+                                    .and_then(|a| serde_json::to_string(&a.scopes).ok()),
+            deliver_json:       h.manifest.auth.as_ref()
+                                    .and_then(|a| a.deliver.as_ref())
+                                    .and_then(|d| serde_json::to_string(d).ok()),
             role_filter:        None,
+            verify_command:     verify_command.as_deref(),
+            verify_script_path: verify_script_path.as_deref(),
+            icon_small_path:    icon_small_path.as_deref(),
+            icon_large_path:    icon_large_path.as_deref(),
             friendly_name:      h.entry.name.as_deref().or(h.manifest.name.as_deref()),
             // The LLM-facing blurb — this is the column `render_mcp_list` puts in
             // the prompt for `activate_tools()`, so the feed's
@@ -641,17 +726,50 @@ pub async fn install(
         "name":           h.entry.id,
         "scope":          scope,
         "source":         source,
-        "files_verified": verified,
+        "files_verified": installed.verified,
     })))
 }
 
-/// Downloads every file the manifest declares into `./scripts//`, refusing any
+/// If a verify `command` references one of the connector's shipped files (by
+/// basename match against `files[]`), return that file's relative path — the
+/// caller stores `/` so the runtime resolves `./connectors//`.
+/// Returns `None` for inline commands (`curl …`) with no script file.
+fn verify_script_of(command: &str, files: &[FileEntry]) -> Option {
+    for f in files {
+        let basename = f.path.rsplit_once('/').map(|(_, b)| b).unwrap_or(&f.path);
+        if !basename.is_empty() && command.contains(basename) {
+            return Some(f.path.clone());
+        }
+    }
+    None
+}
+
+/// What [`download_verified`] put on disk.
+struct Installed {
+    /// How many files were downloaded and matched their declared digest.
+    verified: usize,
+    /// The relative paths actually written, so the caller can record a manifest
+    /// claim (an icon, say) only once the file backing it exists.
+    files:    std::collections::HashSet,
+}
+
+/// Downloads a connector's whole folder into `./connectors//`, refusing any file
 /// whose SHA-256 does not match. All-or-nothing: files are verified in memory and
 /// only written once every digest checks out, so a tampered feed never leaves a
-/// partial connector on disk. Returns how many files were verified.
-async fn download_verified(entry: &IndexEntry, manifest: &Manifest) -> Result {
+/// partial connector on disk.
+///
+/// Runs for **every** source, not just `local_script`. Fetching an icon is not the
+/// §14 risk axis — that axis is about code the box will *execute*, and it stays
+/// gated on `mcp.register_local_script` in [`install`]. What lands here for a remote
+/// connector is inert: an icon and the manifest.
+async fn download_verified(
+    entry:    &IndexEntry,
+    manifest: &Manifest,
+    raw:      Option<&str>,
+    source:   &str,
+) -> Result {
     let files = files_of(entry, manifest);
-    if files.is_empty() {
+    if files.is_empty() && source == "local_script" {
         return Err(ApiError::bad_request(
             "the feed declares no `files` with digests for this connector — \
              refusing to install unverifiable code (§14)",
@@ -660,15 +778,16 @@ async fn download_verified(entry: &IndexEntry, manifest: &Manifest) -> Result)> = Vec::new();
 
     for f in files {
         let rel = safe_rel_path(&f.path)?;
 
-        // Defensive: a document can never carry its own digest (writing the hash
-        // changes the file), so a self-entry is unverifiable by construction. The
-        // feed now keeps digests in the index, where this cannot arise.
-        if rel == "connector.json" {
+        // A document can never carry its own digest (writing the hash into the file
+        // changes the file), so a *manifest*-declared self-entry is unverifiable by
+        // construction. From the index it is verifiable, and gets no exception.
+        if rel == skald_core::mcp::MANIFEST_FILE && !index_digests {
             continue;
         }
 
@@ -703,19 +822,19 @@ async fn download_verified(entry: &IndexEntry, manifest: &Manifest) -> Result = std::collections::HashSet::new();
     for (rel, bytes) in staged {
         let path = dest.join(&rel);
         if let Some(parent) = path.parent() {
@@ -724,8 +843,36 @@ async fn download_verified(entry: &IndexEntry, manifest: &Manifest) -> Result, folder: &str, installed: &Installed) -> Option {
+    let claim = claim?.trim_start_matches('/');
+    let rel = claim.strip_prefix(&format!("{folder}/")).unwrap_or(claim);
+    installed.files.contains(rel).then(|| rel.to_string())
 }
 
 #[cfg(test)]
@@ -837,6 +984,35 @@ mod tests {
         assert_eq!(files_of(&entry(r#"{"id":"x"}"#), &m)[0].path, "b.py");
     }
 
+    /// The index names icons from the feed root (`gmail/icon_sm.svg`) but names
+    /// `files[]` from the connector's folder (`icon_sm.svg`). Recording the wrong one
+    /// would 404 every icon, so this is where the two vocabularies must meet.
+    #[test]
+    fn icon_paths_are_reduced_to_the_connector_folder() {
+        let installed = Installed {
+            verified: 2,
+            files:    ["icon_sm.svg", "server.py"].iter().map(|s| s.to_string()).collect(),
+        };
+        assert_eq!(
+            installed_icon(Some("gmail/icon_sm.svg"), "gmail", &installed),
+            Some("icon_sm.svg".to_string())
+        );
+        // A leading slash is still feed-root-relative.
+        assert_eq!(
+            installed_icon(Some("/gmail/icon_sm.svg"), "gmail", &installed),
+            Some("icon_sm.svg".to_string())
+        );
+        // Already folder-relative: left alone.
+        assert_eq!(
+            installed_icon(Some("icon_sm.svg"), "gmail", &installed),
+            Some("icon_sm.svg".to_string())
+        );
+        // Claimed but never installed → recorded as absent, so the endpoint says
+        // "no icon" instead of pointing the browser at a file that is not there.
+        assert_eq!(installed_icon(Some("gmail/icon_lg.svg"), "gmail", &installed), None);
+        assert_eq!(installed_icon(None, "gmail", &installed), None);
+    }
+
     #[test]
     fn sha256_matches_a_known_vector() {
         assert_eq!(
@@ -845,6 +1021,71 @@ mod tests {
         );
     }
 
+    /// Installs every connector the live feed offers into a throwaway directory and
+    /// checks what actually lands: the digests hold, the manifest is recorded, and the
+    /// icon path stored in the catalog names a file that exists.
+    ///
+    /// That last one is the whole point of the icon column — a path that does not
+    /// resolve would 404 silently in the browser and look like "this connector has no
+    /// icon", which is exactly the failure a unit test with a hand-written feed cannot
+    /// see. `#[ignore]`d (network + it moves the process cwd, so it must run alone):
+    /// `cargo test --bin skald -- --ignored live_feed_installs`.
+    #[tokio::test]
+    #[ignore]
+    async fn live_feed_installs_folder_with_icon_and_manifest() {
+        let _ = rustls::crypto::ring::default_provider().install_default();
+
+        let tmp = std::env::temp_dir().join(format!("skald-install-test-{}", std::process::id()));
+        std::fs::create_dir_all(&tmp).unwrap();
+        std::env::set_current_dir(&tmp).unwrap();
+
+        let feed = match fetch_feed().await {
+            Ok(f) => f,
+            Err(e) => panic!("feed unreachable: {}", e.message),
+        };
+        assert!(!feed.is_empty(), "feed returned no connectors");
+
+        for h in &feed {
+            let source = norm_source(&h.entry, &h.manifest);
+            let installed =
+                download_verified(&h.entry, &h.manifest, h.manifest_raw.as_deref(), &source)
+                    .await
+                    .unwrap_or_else(|e| panic!("`{}` failed to install: {}", h.entry.id, e.message));
+
+            let dir = skald_core::mcp::connector_dir(&h.entry.id).unwrap();
+            let folder = folder_of(&h.entry);
+
+            // Every source installs now — a remote connector included, which is what
+            // gives Tavily an icon at all.
+            assert!(dir.is_dir(), "`{}` installed no folder", h.entry.id);
+
+            // The manifest is recorded even though the feed does not digest it.
+            assert!(
+                dir.join(skald_core::mcp::MANIFEST_FILE).is_file(),
+                "`{}` recorded no connector.json",
+                h.entry.id
+            );
+
+            // The icon path the catalog would store must name a real file.
+            for (size, claim) in [("sm", &h.entry.icon_small), ("lg", &h.entry.icon_large)] {
+                let Some(rel) = installed_icon(claim.as_deref(), &folder, &installed) else {
+                    panic!("`{}` declares a {size} icon the installer did not keep", h.entry.id);
+                };
+                assert!(
+                    dir.join(&rel).is_file(),
+                    "`{}` {size} icon `{rel}` is not on disk",
+                    h.entry.id
+                );
+                // An icon must never be shipped into a user's container.
+                assert!(skald_core::mcp::install::is_host_asset(&rel), "`{rel}` should be a host asset");
+            }
+
+            println!("{:<8} {} files verified, icon + manifest on disk", h.entry.id, installed.verified);
+        }
+
+        std::fs::remove_dir_all(&tmp).ok();
+    }
+
     /// Hits the real feed. `#[ignore]`d so the suite stays offline-clean; run with
     /// `cargo test --bin skald -- --ignored live_feed`.
     #[tokio::test]
diff --git a/src/frontend/api/mcp.rs b/src/frontend/api/mcp.rs
index 0fb1146..8835cd4 100644
--- a/src/frontend/api/mcp.rs
+++ b/src/frontend/api/mcp.rs
@@ -17,7 +17,7 @@ use axum::Json;
 use serde::Deserialize;
 use serde_json::{json, Value};
 
-use skald_core::db::{mcp_catalog, mcp_global_access, mcp_global_servers, mcp_user_servers, role_capabilities};
+use skald_core::db::{mcp_catalog, mcp_global_access, mcp_global_servers, mcp_user_servers, oauth_providers, role_capabilities};
 use skald_core::skald::Skald;
 
 use super::guard::AuthUser;
@@ -40,31 +40,217 @@ fn to_json_opt(v: &Option) -> Option {
     v.as_ref().and_then(|x| serde_json::to_string(x).ok())
 }
 
-/// Copies a vetted catalog script from `./scripts/` into the user's
-/// bind-mounted home under `.skald/mcp//`, and returns the path it will have
-/// INSIDE the container (`/root/.skald/mcp/...`). The home is the only durable
-/// zone (§6), so the script survives a container recreate. Single files only for
-/// now — directory-tree scripts (e.g. whatsapp_mcp/) are a follow-up.
-fn copy_script_into_home(user_id: &str, name: &str, script_path: &str) -> Result {
-    let wd = std::env::current_dir()
-        .map_err(|e| ApiError::bad_request(format!("cannot resolve working directory: {e}")))?;
-    let src = wd.join("scripts").join(script_path);
-    if !src.is_file() {
+/// Installs the connector folder that `script_path` (`/`) belongs
+/// to into the caller's container home, and returns the path the entry file will
+/// have INSIDE the container.
+///
+/// The whole folder travels, not just the entry file — which is what finally gets a
+/// connector's `requirements.txt` and its multi-file trees to where the server
+/// actually runs. The home is the only durable zone (§6), so it survives a
+/// container recreate.
+fn install_connector_for_user(
+    user_id:     &str,
+    name:        &str,
+    script_path: &str,
+) -> Result {
+    let (folder, entry_file) = skald_core::mcp::split_script_path(script_path)
+        .map_err(|e| ApiError::bad_request(e.to_string()))?;
+    let dir = skald_core::mcp::install_into_home(user_id, name, folder)
+        .map_err(|e| ApiError::bad_request(format!("failed to install connector files: {e}")))?
+        .ok_or_else(|| ApiError::bad_request(format!(
+            "connector `{folder}` has no installed files under ./{}/ — \
+             reinstall it from the marketplace",
+            skald_core::mcp::CONNECTORS_DIR,
+        )))?;
+    Ok(dir.join(entry_file).to_string_lossy().into_owned())
+}
+
+// ── verify-before-save helpers ───────────────────────────────────────────────
+
+/// One entry of the catalog's `config_schema_json` (the marketplace `env[]`).
+/// Drives both the activation form (frontend) and the env/secret split (here).
+#[derive(Debug, serde::Deserialize)]
+struct EnvSchemaEntry {
+    name:        String,
+    #[serde(default)] secret: bool,
+    #[allow(dead_code)]
+    #[serde(default)] required: bool,
+}
+
+/// Parses the catalog's `config_schema_json` into schema entries. Tolerates the
+/// legacy `["KEY1","KEY2"]` shape (treated as non-secret) and the new object
+/// array; anything unparseable yields an empty schema.
+fn parse_env_schema(config_schema_json: &Option) -> Vec {
+    let raw = match config_schema_json.as_deref() {
+        Some(s) => s,
+        None => return Vec::new(),
+    };
+    // Object-array form (the new manifest `env[]`).
+    if let Ok(v) = serde_json::from_str::>(raw) {
+        return v;
+    }
+    // Legacy bare-name form.
+    serde_json::from_str::>(raw)
+        .unwrap_or_default()
+        .into_iter()
+        .map(|name| EnvSchemaEntry { name, secret: false, required: false })
+        .collect()
+}
+
+/// Splits the form values into non-secret (`env`) and secret (`secret`) maps,
+/// the two channels [`run_verify`] substitutes into `{ENV:…}` / `{SECRET:…}`.
+///
+/// When `auth_kind == "api_key"`, the supplied `api_key` is also injected under
+/// every secret-name declared by the schema — the canonical case being Tavily,
+/// whose schema names the key `tavilyApiKey` and whose URL carries the matching
+/// `{SECRET:tavilyApiKey}` token.
+fn split_form_values(
+    form: Option<&HashMap>,
+    api_key: Option<&str>,
+    schema: &[EnvSchemaEntry],
+    auth_kind: &str,
+) -> (HashMap, HashMap) {
+    let secret_names: std::collections::HashSet<&str> =
+        schema.iter().filter(|e| e.secret).map(|e| e.name.as_str()).collect();
+
+    let mut env = HashMap::new();
+    let mut secret = HashMap::new();
+    if let Some(form) = form {
+        for (k, v) in form {
+            if secret_names.contains(k.as_str()) {
+                secret.insert(k.clone(), v.clone());
+            } else {
+                env.insert(k.clone(), v.clone());
+            }
+        }
+    }
+
+    // An api_key connector maps the key into every declared secret name that the
+    // form did not already fill (the UI's api_key box is the same value).
+    if auth_kind == "api_key" {
+        if let Some(key) = api_key {
+            for name in secret_names {
+                secret.entry(name.to_string()).or_insert_with(|| key.to_string());
+            }
+        }
+    }
+    (env, secret)
+}
+
+/// Installs the connector folder holding a catalog entry's verify script into the
+/// user's home, returning the in-container directory to run it from. `None` if the
+/// entry declares no verify script.
+///
+/// This runs on the Test button too, before any activation exists — which is why it
+/// installs rather than assuming the folder is already there.
+fn prepare_user_verify_workdir(
+    user_id: &str,
+    name: &str,
+    entry: &mcp_catalog::McpCatalogRow,
+) -> Result, ApiError> {
+    let verify_path = match &entry.verify_script_path {
+        Some(p) => p,
+        None => return Ok(None),
+    };
+    let (folder, _) = skald_core::mcp::split_script_path(verify_path)
+        .map_err(|e| ApiError::bad_request(e.to_string()))?;
+    let dir = skald_core::mcp::install_into_home(user_id, name, folder)
+        .map_err(|e| ApiError::bad_request(format!("failed to install connector files: {e}")))?
+        .ok_or_else(|| ApiError::bad_request(format!(
+            "connector `{folder}` has no installed files — reinstall it from the marketplace"
+        )))?;
+    Ok(Some(dir))
+}
+
+/// The host working dir for a global connector's verify script — the
+/// `./connectors//` directory the marketplace installer populated.
+fn global_verify_workdir(catalog_name: &str) -> Result {
+    let dir = skald_core::mcp::connector_dir(catalog_name)
+        .map_err(|e| ApiError::bad_request(format!("cannot resolve the connectors dir: {e}")))?;
+    if !dir.is_dir() {
         return Err(ApiError::bad_request(format!(
-            "catalog script `scripts/{script_path}` not found or not a file \
-             (directory-tree scripts are not supported yet)"
+            "connector directory `{}/{catalog_name}` not found — reinstall the connector",
+            skald_core::mcp::CONNECTORS_DIR,
         )));
     }
-    let basename = src.file_name()
-        .ok_or_else(|| ApiError::bad_request("invalid script_path"))?
-        .to_string_lossy().to_string();
-    let dest_dir = wd.join(skald_core::container::HOMES_DIR)
-        .join(user_id).join(".skald").join("mcp").join(name);
-    std::fs::create_dir_all(&dest_dir)
-        .map_err(|e| ApiError::bad_request(format!("failed to create script dir: {e}")))?;
-    std::fs::copy(&src, dest_dir.join(&basename))
-        .map_err(|e| ApiError::bad_request(format!("failed to copy script: {e}")))?;
-    Ok(format!("/root/.skald/mcp/{name}/{basename}"))
+    Ok(dir)
+}
+
+/// Default timeout for a connector-declared verify step. (The manifest can also
+/// carry `verify.timeout_secs`; wiring that through is a follow-up.)
+const VERIFY_TIMEOUT_SECS: u64 = 20;
+
+// ── connector icons ───────────────────────────────────────────────────────────
+
+#[derive(Deserialize)]
+pub struct IconQuery {
+    /// `sm` (default) | `lg`.
+    #[serde(default)]
+    pub size: Option,
+}
+
+/// `GET /api/mcp/catalog/{name}/icon?size=sm|lg` — the icon of an **installed**
+/// connector, served off `./connectors//`.
+///
+/// Authenticated but deliberately **not** capability-gated: seeing the icon of a
+/// connector you are allowed to activate is not an administrative act. The
+/// marketplace's own icon endpoint cannot do this job — it proxies the live feed
+/// behind `manage_catalog`, so a normal user gets a 403, and the image would vanish
+/// the moment the feed went down or the entry was pulled upstream. Once installed,
+/// the bytes are ours.
+pub async fn catalog_icon(
+    State(skald): State>,
+    Extension(_auth): Extension,
+    Path(name): Path,
+    axum::extract::Query(q): axum::extract::Query,
+) -> Result {
+    use axum::http::header;
+    use axum::response::IntoResponse;
+
+    let entry = mcp_catalog::get_by_name(skald.db(), &name).await?
+        .ok_or_else(|| ApiError::not_found(format!("no catalog entry `{name}`")))?;
+
+    let large = q.size.as_deref() == Some("lg");
+    let rel = if large {
+        entry.icon_large_path.clone().or_else(|| entry.icon_small_path.clone())
+    } else {
+        entry.icon_small_path.clone().or_else(|| entry.icon_large_path.clone())
+    }
+    .ok_or_else(|| ApiError::not_found("this connector has no installed icon"))?;
+
+    // Containment, mirroring `tools::fs::resolve_host_path`: canonicalize and
+    // prefix-check, fail-closed. `rel` only ever holds a path the installer already
+    // proved safe, and `name` only ever names a real catalog row — this is the belt
+    // to those braces, and the reason a bad row cannot turn into an arbitrary read.
+    let dir = skald_core::mcp::connector_dir(&entry.name)
+        .map_err(|e| ApiError::bad_request(format!("cannot resolve the connectors dir: {e}")))?;
+    let base = dir.canonicalize()
+        .map_err(|_| ApiError::not_found("this connector has no installed files"))?;
+    let path = base.join(&rel).canonicalize()
+        .map_err(|_| ApiError::not_found("icon file is missing — reinstall the connector"))?;
+    if !path.starts_with(&base) {
+        return Err(ApiError::forbidden("icon path escapes the connector directory"));
+    }
+
+    let bytes = std::fs::read(&path)
+        .map_err(|e| ApiError::bad_request(format!("cannot read icon: {e}")))?;
+    let ext = path.extension().and_then(|e| e.to_str()).unwrap_or_default().to_ascii_lowercase();
+    let ct = skald_core::mcp::content_type_for_ext(&ext);
+
+    // An icon is untrusted bytes from a remote feed, and half of them are SVGs —
+    // a format that can carry script. Served from our own origin, that would be
+    // stored XSS for anyone who opened the URL directly. `nosniff` pins the type,
+    // and the CSP neuters any script or fetch the document tries to perform.
+    Ok((
+        [
+            (header::CONTENT_TYPE, ct.to_string()),
+            (header::X_CONTENT_TYPE_OPTIONS, "nosniff".to_string()),
+            (header::CONTENT_SECURITY_POLICY, "default-src 'none'; style-src 'unsafe-inline'".to_string()),
+            (header::CACHE_CONTROL, "private, max-age=3600".to_string()),
+        ],
+        bytes,
+    )
+        .into_response())
 }
 
 // ── existing: running-server introspection ────────────────────────────────────
@@ -100,6 +286,8 @@ pub struct CatalogUpsertBody {
     #[serde(default = "default_none_auth")]
     pub auth_kind:     String,
     pub role_filter:   Option>,
+    pub verify_command:     Option,
+    pub verify_script_path: Option,
     pub friendly_name: Option,
     pub description:   Option,
 }
@@ -130,7 +318,18 @@ pub async fn catalog_upsert(
         script_path:        body.script_path.as_deref(),
         config_schema_json: to_json_opt(&body.config_schema),
         auth_kind:          &body.auth_kind,
+        // OAuth catalog entries come from the vetted feed (marketplace install),
+        // not the admin's manual form — so these stay unset here.
+        oauth_provider:     None,
+        oauth_scopes_json:  None,
+        deliver_json:       None,
         role_filter:        to_json_opt(&body.role_filter),
+        verify_command:     body.verify_command.as_deref(),
+        verify_script_path: body.verify_script_path.as_deref(),
+        // Not the admin form's to set: the installer owns them, and `upsert`
+        // COALESCEs these away rather than blanking an installed connector's icons.
+        icon_small_path:    None,
+        icon_large_path:    None,
         friendly_name:      body.friendly_name.as_deref(),
         description:        body.description.as_deref(),
     }).await?;
@@ -147,6 +346,71 @@ pub async fn catalog_delete(
     Ok(Json(json!({ "ok": true })))
 }
 
+// ── admin: OAuth providers (§15) ──────────────────────────────────────────────
+
+/// The OAuth identity providers, **without** client secrets (never leaves the
+/// process for the browser).
+pub async fn providers_list(
+    State(skald): State>,
+    Extension(auth): Extension,
+) -> Result>, ApiError> {
+    require_cap(&skald, &auth.user_id, role_capabilities::MANAGE_CATALOG).await?;
+    let rows = oauth_providers::list(skald.db()).await?;
+    Ok(Json(rows.into_iter().map(Into::into).collect()))
+}
+
+#[derive(Deserialize)]
+pub struct ProviderUpsertBody {
+    pub name:          String,
+    pub display_name:  String,
+    pub auth_url:      String,
+    pub token_url:     String,
+    pub client_id:     String,
+    /// Empty keeps the stored secret — the list view never gave it back, so editing
+    /// the URLs must not force the admin to re-paste it.
+    #[serde(default)]
+    pub client_secret: String,
+    pub redirect_uri:  String,
+    pub extra_params:  Option,
+}
+
+pub async fn providers_upsert(
+    State(skald): State>,
+    Extension(auth): Extension,
+    Json(body): Json,
+) -> Result, ApiError> {
+    require_cap(&skald, &auth.user_id, role_capabilities::MANAGE_CATALOG).await?;
+    // `extra_params` must be valid JSON — it is merged into the consent URL, and a
+    // malformed value would silently drop `access_type`/`prompt` and cost the user a
+    // refresh token. Reject it here rather than fail quietly at sign-in.
+    if let Some(extra) = body.extra_params.as_deref().filter(|s| !s.trim().is_empty()) {
+        serde_json::from_str::>(extra)
+            .map_err(|e| ApiError::bad_request(format!("extra_params is not a JSON object of strings: {e}")))?;
+    }
+    let extra = body.extra_params.as_deref().filter(|s| !s.trim().is_empty());
+    oauth_providers::upsert(skald.db(), oauth_providers::UpsertProvider {
+        name:          &body.name,
+        display_name:  &body.display_name,
+        auth_url:      &body.auth_url,
+        token_url:     &body.token_url,
+        client_id:     &body.client_id,
+        client_secret: &body.client_secret,
+        redirect_uri:  &body.redirect_uri,
+        extra_params:  extra,
+    }).await?;
+    Ok(Json(json!({ "ok": true })))
+}
+
+pub async fn providers_delete(
+    State(skald): State>,
+    Extension(auth): Extension,
+    Path(name): Path,
+) -> Result, ApiError> {
+    require_cap(&skald, &auth.user_id, role_capabilities::MANAGE_CATALOG).await?;
+    oauth_providers::delete(skald.db(), &name).await?;
+    Ok(Json(json!({ "ok": true })))
+}
+
 // ── admin: globally-active connectors + access ────────────────────────────────
 
 pub async fn global_list(
@@ -181,25 +445,37 @@ pub async fn global_enable(
     let name = body.name.clone().unwrap_or_else(|| entry.name.clone());
     // Snapshot the concrete config from the catalog; the admin supplies the secret.
     let id = mcp_global_servers::upsert(skald.db(), mcp_global_servers::UpsertGlobal {
-        name:          &name,
-        catalog_name:  Some(&entry.name),
-        transport:     &entry.transport,
-        command:       entry.command.as_deref(),
-        args_json:     entry.args_json.clone(),
-        env_json:      body.env.as_ref().and_then(|e| serde_json::to_string(e).ok()).or_else(|| entry.env_json.clone()),
-        url:           entry.url.as_deref(),
-        api_key:       body.api_key.as_deref(),
-        friendly_name: entry.friendly_name.as_deref(),
-        description:   entry.description.as_deref(),
+        name:               &name,
+        catalog_name:       Some(&entry.name),
+        transport:          &entry.transport,
+        command:            entry.command.as_deref(),
+        args_json:          entry.args_json.clone(),
+        env_json:           body.env.as_ref().and_then(|e| serde_json::to_string(e).ok()).or_else(|| entry.env_json.clone()),
+        url:                entry.url.as_deref(),
+        api_key:            body.api_key.as_deref(),
+        verify_command:     entry.verify_command.as_deref(),
+        verify_script_path: entry.verify_script_path.as_deref(),
+        friendly_name:      entry.friendly_name.as_deref(),
+        description:        entry.description.as_deref(),
     }).await?;
 
+    // Verify the admin-supplied credentials before starting the server. A failure
+    // disables the row so it does not run with bad creds; the admin sees the
+    // message and can fix + re-enable. A connector with no verify step is allowed
+    // through unchanged.
+    let verify = run_verify_for_entry(&skald, &auth, &entry, &name, body.env.as_ref(), body.api_key.as_deref()).await?;
+    if !verify.skipped && !verify.ok {
+        mcp_global_servers::set_enabled(skald.db(), id, false).await?;
+        return Ok(Json(json!({ "id": id, "verify": verify, "error": verify.message })));
+    }
+
     // Start it now in the global runtime (host transport).
     let row = mcp_global_servers::get(skald.db(), id).await?
         .ok_or_else(|| ApiError::bad_request("global server vanished after upsert"))?;
     let spec = skald_core::mcp::global_row_spec(&row);
     match skald.mcp().start_server(spec).await {
-        Ok(tools) => Ok(Json(json!({ "id": id, "tools": tools }))),
-        Err(e)    => Ok(Json(json!({ "id": id, "error": e.to_string() }))),
+        Ok(tools) => Ok(Json(json!({ "id": id, "tools": tools, "verify": verify }))),
+        Err(e)    => Ok(Json(json!({ "id": id, "error": e.to_string(), "verify": verify }))),
     }
 }
 
@@ -216,6 +492,98 @@ pub async fn global_delete(
     Ok(Json(json!({ "ok": true })))
 }
 
+/// Runs a catalog entry's verify step in the right target — inside the caller's
+/// container for a `per_user` local_script, on the host for a `global` remote.
+/// Returns [`VerifyReport::skipped`] when the entry declares no verify step, so
+/// callers can treat "no test" uniformly.
+///
+/// `runtime_name` is the in-container directory key (the user's chosen runtime
+/// name for an activation, or the catalog name for a standalone Test).
+async fn run_verify_for_entry(
+    skald: &Skald,
+    auth: &AuthUser,
+    entry: &mcp_catalog::McpCatalogRow,
+    runtime_name: &str,
+    form: Option<&HashMap>,
+    api_key: Option<&str>,
+) -> Result {
+    use skald_core::mcp::{run_verify, VerifyReport, VerifyTarget};
+
+    let verify_command = match entry.verify_command.as_deref() {
+        Some(c) => c,
+        None => return Ok(VerifyReport::skipped()),
+    };
+    let schema = parse_env_schema(&entry.config_schema_json);
+    let (env_values, secret_values) = split_form_values(form, api_key, &schema, &entry.auth_kind);
+
+    let report = match entry.scope.as_str() {
+        "per_user" => {
+            // `require_context` ensures the container exists before we exec into it.
+            let _ctx = require_context(skald, &auth.user_id).await?;
+            let workdir = prepare_user_verify_workdir(&auth.user_id, runtime_name, entry)?
+                .ok_or_else(|| ApiError::bad_request(
+                    "catalog entry declares verify_command but no verify_script_path",
+                ))?;
+            let container = skald_core::container::container_name(&auth.user_id);
+            run_verify(
+                verify_command,
+                &env_values,
+                &secret_values,
+                VerifyTarget::Container { container: &container, workdir: &workdir },
+                VERIFY_TIMEOUT_SECS,
+            ).await
+        }
+        "global" => {
+            require_cap(skald, &auth.user_id, role_capabilities::MANAGE_CATALOG).await?;
+            let workdir = global_verify_workdir(&entry.name)?;
+            run_verify(
+                verify_command,
+                &env_values,
+                &secret_values,
+                VerifyTarget::Host { workdir: &workdir },
+                VERIFY_TIMEOUT_SECS,
+            ).await
+        }
+        other => return Err(ApiError::bad_request(format!("unknown catalog scope `{other}`"))),
+    };
+    Ok(report)
+}
+
+// ── user: test a connector without persisting ─────────────────────────────────
+
+#[derive(Deserialize)]
+pub struct TestBody {
+    /// The catalog entry whose credentials to probe.
+    pub catalog_name: String,
+    /// The form values the user just typed (env + secret mixed).
+    pub env:          Option>,
+    pub api_key:      Option,
+}
+
+/// `POST /api/mcp/test` — runs a connector's verify step with the supplied
+/// credentials and returns the [`VerifyReport`] **without persisting anything**.
+/// The frontend Test button calls this before offering Activate.
+pub async fn test(
+    State(skald): State>,
+    Extension(auth): Extension,
+    Json(body): Json,
+) -> Result, ApiError> {
+    let entry = mcp_catalog::get_by_name(skald.db(), &body.catalog_name).await?
+        .ok_or_else(|| ApiError::not_found(format!("no catalog entry `{}`", body.catalog_name)))?;
+    // Per-role gate, mirroring `activate`: a user may only test connectors
+    // their role is allowed to activate.
+    let user = skald_core::db::users::get(skald.db(), &auth.user_id).await?
+        .ok_or_else(|| ApiError::unauthorized("unknown user"))?;
+    if !entry.allowed_for_role(&user.role_id) {
+        return Err(ApiError::forbidden("your role may not use this connector"));
+    }
+    let report = run_verify_for_entry(
+        &skald, &auth, &entry, &entry.name,
+        body.env.as_ref(), body.api_key.as_deref(),
+    ).await?;
+    Ok(Json(report))
+}
+
 #[derive(Deserialize)]
 pub struct GlobalAccessBody {
     /// The full set of user ids allowed to use this global connector.
@@ -369,29 +737,79 @@ pub async fn activate(
             let name = body.name.clone().unwrap_or_else(|| entry.name.clone());
             reject_name_collision(&skald, &ctx.pool, &auth.user_id, &name).await?;
 
-            // For a local script, copy it into the container home and point the
-            // command at the in-container path.
+            // For a local script, install its folder into the container home and
+            // point the command at the in-container path.
             let (command, args_json, script_rel_path) = if entry.source == "local_script" {
                 let script = entry.script_path.clone()
                     .ok_or_else(|| ApiError::bad_request("catalog local_script entry has no script_path"))?;
-                let container_path = copy_script_into_home(&auth.user_id, &name, &script)?;
+                let container_path = install_connector_for_user(&auth.user_id, &name, &script)?;
                 (entry.command.clone(), Some(json!([container_path]).to_string()), Some(container_path))
             } else {
                 (entry.command.clone(), entry.args_json.clone(), None)
             };
+            let env_json = body.env.as_ref()
+                .and_then(|e| serde_json::to_string(e).ok())
+                .or_else(|| entry.env_json.clone());
+
+            // OAuth connectors do NOT activate directly (§15): the refresh token
+            // comes from an interactive consent, not from the activation form. We
+            // persist a PENDING row (files installed, command wired) and hand off to
+            // `/mcp/oauth/start` → `/complete`, which obtains the token, flips the
+            // row to `ready`, and starts the server. Nothing runs until then.
+            if entry.auth_kind == "oauth" {
+                let provider_name = entry.oauth_provider.as_deref().ok_or_else(|| {
+                    ApiError::bad_request("this OAuth connector names no provider in the catalog")
+                })?;
+                // Fail early, clearly, if the admin has not configured the provider —
+                // better than a dead pending row the user cannot complete.
+                let provider = skald_core::db::oauth_providers::get(skald.db(), provider_name).await?
+                    .ok_or_else(|| ApiError::bad_request(format!(
+                        "the `{provider_name}` sign-in provider is not set up yet — \
+                         an admin must add its client credentials first"
+                    )))?;
+                if provider.client_id.is_empty() {
+                    return Err(ApiError::bad_request(format!(
+                        "the `{provider_name}` sign-in provider has no client id configured"
+                    )));
+                }
+                let id = mcp_user_servers::insert(&ctx.pool, mcp_user_servers::InsertUserServer {
+                    name:                   &name,
+                    catalog_name:           Some(&entry.name),
+                    source:                 &entry.source,
+                    transport:              &entry.transport,
+                    command:                command.as_deref(),
+                    args_json,
+                    env_json,
+                    url:                    entry.url.as_deref(),
+                    api_key:                None,                 // obtained by the OAuth flow
+                    oauth_provider:         Some(provider_name),
+                    deliver_json:           entry.deliver_json.clone(),
+                    script_rel_path:        script_rel_path.as_deref(),
+                    verify_command:         None,
+                    verify_script_rel_path: None,
+                    auth_state:             "pending",
+                }).await?;
+                return Ok(Json(json!({
+                    "id": id, "auth_state": "pending", "needs_oauth": true,
+                })));
+            }
 
             mcp_user_servers::insert(&ctx.pool, mcp_user_servers::InsertUserServer {
-                name:            &name,
-                catalog_name:    Some(&entry.name),
-                source:          &entry.source,
-                transport:       &entry.transport,
-                command:         command.as_deref(),
+                name:                   &name,
+                catalog_name:           Some(&entry.name),
+                source:                 &entry.source,
+                transport:              &entry.transport,
+                command:                command.as_deref(),
                 args_json,
-                env_json:        body.env.as_ref().and_then(|e| serde_json::to_string(e).ok()).or_else(|| entry.env_json.clone()),
-                url:             entry.url.as_deref(),
-                api_key:         body.api_key.as_deref(),
-                script_rel_path: script_rel_path.as_deref(),
-                auth_state:      "ready",
+                env_json,
+                url:                    entry.url.as_deref(),
+                api_key:                body.api_key.as_deref(),
+                oauth_provider:         None,
+                deliver_json:           None,
+                script_rel_path:        script_rel_path.as_deref(),
+                verify_command:         entry.verify_command.as_deref(),
+                verify_script_rel_path: None, // resolved when verify runs in-container
+                auth_state:             "ready",
             }).await?
         }
         None => {
@@ -403,17 +821,21 @@ pub async fn activate(
                 .ok_or_else(|| ApiError::bad_request("a self-registered remote needs a `url`"))?;
             reject_name_collision(&skald, &ctx.pool, &auth.user_id, &name).await?;
             mcp_user_servers::insert(&ctx.pool, mcp_user_servers::InsertUserServer {
-                name:            &name,
-                catalog_name:    None,
-                source:          "remote",
-                transport:       &body.transport,
-                command:         None,
-                args_json:       None,
-                env_json:        body.env.as_ref().and_then(|e| serde_json::to_string(e).ok()),
-                url:             Some(&url),
-                api_key:         body.api_key.as_deref(),
-                script_rel_path: None,
-                auth_state:      "ready",
+                name:                   &name,
+                catalog_name:           None,
+                source:                 "remote",
+                transport:              &body.transport,
+                command:                None,
+                args_json:              None,
+                env_json:               body.env.as_ref().and_then(|e| serde_json::to_string(e).ok()),
+                url:                    Some(&url),
+                api_key:                body.api_key.as_deref(),
+                oauth_provider:         None,
+                deliver_json:           None,
+                script_rel_path:        None,
+                verify_command:         None,
+                verify_script_rel_path: None,
+                auth_state:             "ready",
             }).await?
         }
     };
@@ -421,11 +843,44 @@ pub async fn activate(
     // Start it now in this user's runtime (container transport for stdio).
     let row = mcp_user_servers::get(&ctx.pool, insert).await?
         .ok_or_else(|| ApiError::bad_request("user server vanished after insert"))?;
+
+    // Verify-before-start for catalog local_script connectors (a self-registered
+    // remote has no verify step). The activation is persisted either way; a
+    // failed verify flips auth_state to 'pending' so the user can retry without
+    // retyping, and the row stays out of `all_startable` until a test passes.
+    let verify = if row.source == "local_script" && row.verify_command.is_some() {
+        match &row.catalog_name {
+            Some(catalog_name) => match mcp_catalog::get_by_name(skald.db(), catalog_name).await? {
+                Some(entry) => {
+                    let report = run_verify_for_entry(
+                        &skald, &auth, &entry, &row.name,
+                        body.env.as_ref(), body.api_key.as_deref(),
+                    ).await?;
+                    if !report.skipped && !report.ok {
+                        mcp_user_servers::set_auth_state(&ctx.pool, insert, "pending").await?;
+                        return Ok(Json(json!({
+                            "id": insert, "verify": report, "auth_state": "pending",
+                        })));
+                    }
+                    report
+                }
+                None => skald_core::mcp::VerifyReport::skipped(),
+            },
+            None => skald_core::mcp::VerifyReport::skipped(),
+        }
+    } else {
+        skald_core::mcp::VerifyReport::skipped()
+    };
+
     let container = skald_core::container::container_name(&auth.user_id);
-    let spec = skald_core::mcp::user_row_spec(&row, &container);
+    let spec = skald_core::mcp::user_row_spec_resolved(&row, &container, skald.db()).await;
     match ctx.user_mcp.start_server(spec).await {
-        Ok(tools) => Ok(Json(json!({ "id": insert, "tools": tools }))),
-        Err(e)    => Ok(Json(json!({ "id": insert, "error": e.to_string() }))),
+        Ok(tools) => Ok(Json(json!({
+            "id": insert, "tools": tools, "verify": verify, "auth_state": "ready",
+        }))),
+        Err(e) => Ok(Json(json!({
+            "id": insert, "error": e.to_string(), "verify": verify,
+        }))),
     }
 }
 
@@ -460,3 +915,133 @@ pub async fn deactivate(
     mcp_user_servers::delete(&ctx.pool, id).await?;
     Ok(Json(json!({ "ok": true })))
 }
+
+// ── user: interactive OAuth login for a per-user connector (§15) ───────────────
+//
+// A pending consent lives only in RAM, keyed by an opaque `state`: it holds the
+// PKCE verifier that a copy-pasteable authorization code is worthless without, plus
+// which user + connector row it belongs to. The flow is stateless on disk — an
+// abandoned consent is simply pruned, and a restart drops every in-flight flow (the
+// user just starts again), mirroring the RAM-only session model.
+
+struct PendingFlow {
+    user_id:       String,
+    server_id:     i64,
+    verifier:      String,
+    provider_name: String,
+    created_at:    std::time::Instant,
+}
+
+/// How long a started-but-uncompleted consent stays valid.
+const OAUTH_FLOW_TTL: std::time::Duration = std::time::Duration::from_secs(15 * 60);
+
+static OAUTH_FLOWS: std::sync::LazyLock>> =
+    std::sync::LazyLock::new(|| std::sync::Mutex::new(HashMap::new()));
+
+/// Stores a pending flow, pruning any that timed out first. The lock is never held
+/// across an `.await` — a single synchronous critical section.
+fn oauth_flow_insert(state: String, flow: PendingFlow) {
+    let mut map = OAUTH_FLOWS.lock().unwrap();
+    map.retain(|_, f| f.created_at.elapsed() < OAUTH_FLOW_TTL);
+    map.insert(state, flow);
+}
+
+/// Removes and returns a pending flow, or `None` if unknown or expired.
+fn oauth_flow_take(state: &str) -> Option {
+    let mut map = OAUTH_FLOWS.lock().unwrap();
+    let flow = map.remove(state)?;
+    (flow.created_at.elapsed() < OAUTH_FLOW_TTL).then_some(flow)
+}
+
+#[derive(Deserialize)]
+pub struct OauthStartBody {
+    /// The pending `mcp_user_servers` row (created by `activate`) to sign in.
+    pub server_id: i64,
+}
+
+/// `POST /api/mcp/oauth/start` — begins the consent for a pending OAuth connector.
+/// Returns the URL the user opens and an opaque `state` the caller echoes back to
+/// `/complete`. Does not touch the provider or the network beyond building a URL.
+pub async fn oauth_start(
+    State(skald): State>,
+    Extension(auth): Extension,
+    Json(body): Json,
+) -> Result, ApiError> {
+    let ctx = require_context(&skald, &auth.user_id).await?;
+    let row = mcp_user_servers::get(&ctx.pool, body.server_id).await?
+        .ok_or_else(|| ApiError::not_found("no such connector"))?;
+    let provider_name = row.oauth_provider.as_deref()
+        .ok_or_else(|| ApiError::bad_request("this connector does not use OAuth"))?;
+    let provider = skald_core::db::oauth_providers::get(skald.db(), provider_name).await?
+        .ok_or_else(|| ApiError::bad_request(format!("sign-in provider `{provider_name}` is not configured")))?;
+
+    // The scopes to request live on the catalog entry (kept current), linked by the
+    // row's snapshotted catalog name.
+    let scopes = match &row.catalog_name {
+        Some(cn) => mcp_catalog::get_by_name(skald.db(), cn).await?
+            .map(|e| e.oauth_scopes())
+            .unwrap_or_default(),
+        None => Vec::new(),
+    };
+
+    let pkce = skald_core::mcp::oauth::generate_pkce();
+    let state = skald_core::mcp::oauth::random_state();
+    let auth_url = skald_core::mcp::oauth::build_consent_url(&provider, &scopes, &state, &pkce.challenge)
+        .map_err(|e| ApiError::bad_request(e.to_string()))?;
+
+    oauth_flow_insert(state.clone(), PendingFlow {
+        user_id:       auth.user_id.clone(),
+        server_id:     row.id,
+        verifier:      pkce.verifier,
+        provider_name: provider_name.to_string(),
+        created_at:    std::time::Instant::now(),
+    });
+
+    Ok(Json(json!({ "auth_url": auth_url, "state": state })))
+}
+
+#[derive(Deserialize)]
+pub struct OauthCompleteBody {
+    /// The `state` returned by `/start`, identifying the pending flow.
+    pub state: String,
+    /// The authorization code the user pasted from the provider's page.
+    pub code:  String,
+}
+
+/// `POST /api/mcp/oauth/complete` — exchanges the pasted code for a refresh token,
+/// stores it, flips the connector to `ready`, and starts it in the user's runtime.
+pub async fn oauth_complete(
+    State(skald): State>,
+    Extension(auth): Extension,
+    Json(body): Json,
+) -> Result, ApiError> {
+    let ctx = require_context(&skald, &auth.user_id).await?;
+    let flow = oauth_flow_take(&body.state)
+        .ok_or_else(|| ApiError::bad_request("this sign-in has expired — start it again"))?;
+    // The flow is bound to the user who started it: a leaked `state` cannot let
+    // someone finish another person's consent into their own connector.
+    if flow.user_id != auth.user_id {
+        return Err(ApiError::forbidden("this sign-in does not belong to you"));
+    }
+    let provider = skald_core::db::oauth_providers::get(skald.db(), &flow.provider_name).await?
+        .ok_or_else(|| ApiError::bad_request("sign-in provider is no longer configured"))?;
+
+    let token = skald_core::mcp::oauth::exchange_code(&provider, body.code.trim(), &flow.verifier)
+        .await
+        .map_err(|e| ApiError::bad_request(e.to_string()))?;
+    let refresh = token.refresh_token.ok_or_else(|| ApiError::bad_request(
+        "the provider returned no refresh token — revoke this app's access in your \
+         account settings and try the sign-in again",
+    ))?;
+    mcp_user_servers::set_oauth_token(&ctx.pool, flow.server_id, &refresh).await?;
+
+    // Start it now, with the credential resolved into the server's env (§15).
+    let row = mcp_user_servers::get(&ctx.pool, flow.server_id).await?
+        .ok_or_else(|| ApiError::bad_request("connector vanished after sign-in"))?;
+    let container = skald_core::container::container_name(&auth.user_id);
+    let spec = skald_core::mcp::user_row_spec_resolved(&row, &container, skald.db()).await;
+    match ctx.user_mcp.start_server(spec).await {
+        Ok(tools) => Ok(Json(json!({ "id": row.id, "tools": tools, "auth_state": "ready" }))),
+        Err(e)    => Ok(Json(json!({ "id": row.id, "error": e.to_string(), "auth_state": "ready" }))),
+    }
+}
diff --git a/src/frontend/api/mod.rs b/src/frontend/api/mod.rs
index db386be..cee4562 100644
--- a/src/frontend/api/mod.rs
+++ b/src/frontend/api/mod.rs
@@ -139,14 +139,24 @@ pub fn router() -> Router> {
         // admin: catalog + globally-active connectors
         .route("/mcp/catalog",                  get(mcp::catalog_list).post(mcp::catalog_upsert))
         .route("/mcp/catalog/{id}",             delete(mcp::catalog_delete))
+        // The icon of an installed connector, off the local `connectors/` folder.
+        // Any logged-in user, not just a catalog manager — see `catalog_icon`.
+        .route("/mcp/catalog/{name}/icon",      get(mcp::catalog_icon))
         .route("/mcp/global",                   get(mcp::global_list).post(mcp::global_enable))
         .route("/mcp/global/{id}",              delete(mcp::global_delete))
         .route("/mcp/global/{id}/access",       get(mcp::global_get_access).put(mcp::global_set_access))
+        // admin: OAuth providers (client credentials for per-user sign-in, §15)
+        .route("/mcp/providers",                get(mcp::providers_list).post(mcp::providers_upsert))
+        .route("/mcp/providers/{name}",         delete(mcp::providers_delete))
         // user: available catalog + per-user activation
         .route("/mcp/available",                get(mcp::available))
         .route("/mcp/activate",                 post(mcp::activate))
+        .route("/mcp/test",                     post(mcp::test))
         .route("/mcp/activated",                get(mcp::activated_list))
         .route("/mcp/activated/{id}",           delete(mcp::deactivate))
+        // user: interactive OAuth login for a pending per-user connector (§15)
+        .route("/mcp/oauth/start",              post(mcp::oauth_start))
+        .route("/mcp/oauth/complete",           post(mcp::oauth_complete))
         // Dev / debug
         .route("/dev/debug_mode",               get(dev::get_debug_mode).post(dev::set_debug_mode).put(dev::set_debug_mode))
         .route("/dev/llm-requests",             get(dev::list_llm_requests))
diff --git a/web/app.js b/web/app.js
index eb633f2..ff8483e 100644
--- a/web/app.js
+++ b/web/app.js
@@ -12,6 +12,7 @@ import { AgentsPage }         from './components/agents.js';
 import { UsersPage }          from './components/users-page.js';
 import { RolesPage }          from './components/roles-page.js';
 import { ConnectorsPage }     from './components/connectors.js';
+import { ConnectorDetailPage } from './components/connector-detail.js';
 import { MarketplacePage }    from './components/marketplace.js';
 import { CatalogPage }        from './components/catalog.js';
 import { ProfilePage }        from './components/profile-page.js';
@@ -46,6 +47,7 @@ customElements.define('agents-page',          AgentsPage);
 customElements.define('users-page',           UsersPage);
 customElements.define('roles-page',           RolesPage);
 customElements.define('connectors-page',      ConnectorsPage);
+customElements.define('connector-detail-page', ConnectorDetailPage);
 customElements.define('marketplace-page',     MarketplacePage);
 customElements.define('catalog-page',         CatalogPage);
 customElements.define('profile-page',         ProfilePage);
diff --git a/web/components/catalog.js b/web/components/catalog.js
index e3146db..e4ca2da 100644
--- a/web/components/catalog.js
+++ b/web/components/catalog.js
@@ -301,7 +301,7 @@ export class CatalogPage extends LightElement {
             ${this._select('Transport', f.transport, ['stdio', 'http', 'sse'], e => this._patch('transport', e.target.value))}
             ${isScript
               ? html`${this._field('Command', f.command, e => this._patch('command', e.target.value), { placeholder: 'python3', mono: true })}
-                     ${this._field('Script path', f.script_path, e => this._patch('script_path', e.target.value), { hint: 'under ./scripts', mono: true })}`
+                     ${this._field('Script path', f.script_path, e => this._patch('script_path', e.target.value), { hint: 'as /, under ./connectors', mono: true })}`
               : this._field('URL', f.url, e => this._patch('url', e.target.value), { mono: true })}
             ${this._field('Args', f.args, e => this._patch('args', e.target.value), { hint: 'one per line', mono: true })}
             ${this._field('Required secret/env keys', f.config_schema, e => this._patch('config_schema', e.target.value), { hint: 'comma/newline', mono: true })}
diff --git a/web/components/connector-detail.js b/web/components/connector-detail.js
new file mode 100644
index 0000000..a20eb8d
--- /dev/null
+++ b/web/components/connector-detail.js
@@ -0,0 +1,626 @@
+import { html, nothing } from 'lit';
+import { LightElement } from '../lib/base.js';
+import {
+  announceChange, connectorIconUrl, jf, normalizeSchema, parseJson, seedEnv, statusOf,
+} from './shared/connector-common.js';
+
+// One connector's own page — `#connector?name=`.
+//
+// This replaces the activation dialog. A connector declares its own env/secret
+// schema, so the form's height is the *connector's* choice, not the UI's: EMAIL asks
+// for a dozen fields, and a fixed-size modal simply could not hold them — it grew
+// taller than the viewport and the buttons went off-screen. A page scrolls.
+//
+// It is also the natural home for everything else that is per-connector and was
+// scattered before: the Test button, the global enable, and the per-user access
+// grants — which used to be a second modal reached from a third place.
+//
+// Deliberately not a `name` field: the list is one row per connector (§7 template),
+// so the runtime name is the catalog name. The backend still defends against
+// collisions; the UI just stops offering a way to cause them.
+
+const ADMIN_ID = 'admin';
+const PAGE_ID  = 'connector';
+
+function nameFromHash() {
+  const m = location.hash.match(/^#connector\?name=(.*)$/);
+  if (!m) return null;
+  try { return decodeURIComponent(m[1]); } catch { return null; }
+}
+
+export class ConnectorDetailPage extends LightElement {
+
+  static get properties() {
+    return {
+      _open:      { state: true },
+      _name:      { state: true },
+      _me:        { state: true },
+      _entry:     { state: true },   // catalog row (null for a global we cannot read)
+      _act:       { state: true },   // my activation row, if any
+      _glob:      { state: true },   // the global instance, if any
+      _schema:    { state: true },
+      _form:      { state: true },   // { api_key, env: {} }
+      _test:      { state: true },   // null | 'running' | report
+      _busy:      { state: true },
+      _error:     { state: true },
+      _users:     { state: true },   // admin: for the access panel
+      _access:    { state: true },   // admin: Set of granted user ids
+      _noIcon:    { state: true },
+      _oauth:     { state: true },   // in-flight OAuth login: { state, auth_url, code }
+    };
+  }
+
+  constructor() {
+    super();
+    this._open = false;
+    this._noIcon = false;
+    this._reset();
+  }
+
+  _reset() {
+    this._name   = null;
+    this._me     = null;
+    this._entry  = null;
+    this._act    = null;
+    this._glob   = null;
+    this._schema = [];
+    this._form   = { api_key: '', env: {} };
+    this._test   = null;
+    this._busy   = false;
+    this._error  = null;
+    this._users  = null;
+    this._access = null;
+    this._oauth  = null;
+  }
+
+  connectedCallback() {
+    super.connectedCallback();
+    window.addEventListener('llm-page-change', (e) => {
+      this._open = e.detail.page === PAGE_ID;
+      this.style.display = this._open ? 'flex' : 'none';
+      if (this._open) this._loadFromHash();
+    });
+    window.addEventListener('hashchange', () => {
+      if (this._open) this._loadFromHash();
+    });
+  }
+
+  get _isAdmin()  { return this._me?.role_id === ADMIN_ID; }
+  get _isGlobal() { return (this._entry?.scope ?? (this._glob ? 'global' : null)) === 'global'; }
+  get _status()   { return statusOf({ _act: this._act, _glob: this._glob }); }
+
+  async _loadFromHash() {
+    const name = nameFromHash();
+    if (!name) return;
+    // A different connector must not inherit the previous one's typed secrets.
+    if (name !== this._name) this._reset();
+    this._name = name;
+    await this._load();
+  }
+
+  async _load() {
+    this._error = null;
+    try {
+      this._me = await jf('/api/auth/me');
+      const [available, activated] = await Promise.all([
+        jf('/api/mcp/available'),
+        jf('/api/mcp/activated'),
+      ]);
+      const entry = (available?.catalog ?? []).find(e => e.name === this._name) ?? null;
+      const glob  = (available?.globals ?? [])
+        .find(g => (g.catalog_name ?? g.name) === this._name) ?? null;
+      const act   = (activated ?? []).find(r => r.catalog_name === this._name) ?? null;
+
+      if (!entry && !glob) {
+        this._error = `No connector named “${this._name}” is available to you.`;
+        return;
+      }
+      this._entry = entry;
+      this._glob  = glob;
+      this._act   = act;
+
+      const schema = normalizeSchema(parseJson(entry?.config_schema_json, []));
+      this._schema = schema;
+      // Keep whatever the user has already typed across a reload triggered by a save.
+      this._form = { api_key: this._form.api_key || '', env: { ...seedEnv(schema), ...this._form.env } };
+
+      if (this._isAdmin && this._isGlobal) await this._loadAccess();
+    } catch (e) {
+      this._error = e.message;
+    }
+  }
+
+  async _loadAccess() {
+    try {
+      this._users = await jf('/api/users');
+      if (this._glob) {
+        const granted = await jf(`/api/mcp/global/${this._glob.id}/access`);
+        this._access = new Set(granted || []);
+      }
+    } catch (e) { this._error = e.message; }
+  }
+
+  _back() {
+    // Prefer real history so the browser's own Back stays consistent; fall back to
+    // the list when this page was opened straight from a pasted URL.
+    if (history.length > 1) { history.back(); return; }
+    history.pushState({ page: 'connectors' }, '', '#connectors');
+    window.dispatchEvent(new CustomEvent('llm-page-change', { detail: { page: 'connectors' } }));
+  }
+
+  _patchEnv(key, value) {
+    this._form = { ...this._form, env: { ...this._form.env, [key]: value } };
+  }
+
+  /// The env map to send: empty fields are dropped so a blank box means "unset"
+  /// rather than "set to empty string".
+  get _envPayload() {
+    const env = {};
+    for (const [k, v] of Object.entries(this._form.env || {})) if (v !== '') env[k] = v;
+    return Object.keys(env).length ? env : null;
+  }
+
+  // ── Actions ────────────────────────────────────────────────────────────────
+
+  async _testCreds() {
+    this._test = 'running';
+    try {
+      this._test = await jf('/api/mcp/test', {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify({
+          catalog_name: this._name,
+          api_key: this._form.api_key || null,
+          env: this._envPayload,
+        }),
+      });
+    } catch (e) {
+      this._test = { ok: false, message: e.message };
+    }
+  }
+
+  async _activate() {
+    this._busy = true; this._error = null;
+    try {
+      const res = await jf('/api/mcp/activate', {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify({
+          catalog_name: this._name,
+          api_key: this._form.api_key || null,
+          env: this._envPayload,
+        }),
+      });
+      if (res?.auth_state === 'pending') {
+        this._test  = res.verify ?? { ok: false, message: 'Verification failed.' };
+        this._error = 'Saved, but the credentials did not check out — fix them and test again.';
+      } else if (res?.error) {
+        this._error = res.error;
+      }
+      announceChange();
+      await this._load();
+    } catch (e) { this._error = e.message; }
+    finally { this._busy = false; }
+  }
+
+  async _deactivate() {
+    if (!confirm(`Deactivate “${this._entry?.friendly_name || this._name}”?`)) return;
+    this._busy = true;
+    try {
+      await jf(`/api/mcp/activated/${this._act.id}`, { method: 'DELETE' });
+      this._act = null;
+      this._test = null;
+      announceChange();
+      await this._load();
+    } catch (e) { this._error = e.message; }
+    finally { this._busy = false; }
+  }
+
+  // ── OAuth login (§15): activate → consent in a tab → paste code → complete ────
+
+  async _startOauth() {
+    this._busy = true; this._error = null;
+    try {
+      // The activation may not exist yet (first sign-in) — create the pending row,
+      // then reuse it. A `pending` row from a previous attempt is signed in again.
+      let serverId = this._act?.id;
+      if (!serverId) {
+        const res = await jf('/api/mcp/activate', {
+          method: 'POST', headers: { 'Content-Type': 'application/json' },
+          body: JSON.stringify({ catalog_name: this._name }),
+        });
+        if (res?.error) { this._error = res.error; return; }
+        serverId = res.id;
+      }
+      const start = await jf('/api/mcp/oauth/start', {
+        method: 'POST', headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify({ server_id: serverId }),
+      });
+      this._oauth = { state: start.state, auth_url: start.auth_url, code: '' };
+      window.open(start.auth_url, '_blank', 'noopener');
+      announceChange();
+      await this._load();
+    } catch (e) { this._error = e.message; }
+    finally { this._busy = false; }
+  }
+
+  async _completeOauth() {
+    this._busy = true; this._error = null;
+    try {
+      const res = await jf('/api/mcp/oauth/complete', {
+        method: 'POST', headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify({ state: this._oauth.state, code: this._oauth.code.trim() }),
+      });
+      if (res?.error) { this._error = res.error; }
+      else { this._oauth = null; }
+      announceChange();
+      await this._load();
+    } catch (e) { this._error = e.message; }
+    finally { this._busy = false; }
+  }
+
+  async _enableGlobal() {
+    this._busy = true; this._error = null;
+    try {
+      const res = await jf('/api/mcp/global', {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify({
+          catalog_name: this._name,
+          api_key: this._form.api_key || null,
+          env: this._envPayload,
+        }),
+      });
+      if (res?.verify && !res.verify.ok && !res.verify.skipped) {
+        this._test  = res.verify;
+        this._error = 'Verification failed — the connector stays disabled until the credentials are fixed.';
+      } else if (res?.error) {
+        this._error = res.error;
+      }
+      announceChange();
+      await this._load();
+    } catch (e) { this._error = e.message; }
+    finally { this._busy = false; }
+  }
+
+  async _disableGlobal() {
+    if (!confirm(`Disable “${this._glob.friendly_name || this._name}”?\n\nIt stops for everyone who can use it.`)) return;
+    this._busy = true;
+    try {
+      await jf(`/api/mcp/global/${this._glob.id}`, { method: 'DELETE' });
+      this._glob = null;
+      announceChange();
+      await this._load();
+    } catch (e) { this._error = e.message; }
+    finally { this._busy = false; }
+  }
+
+  _toggleAccess(userId) {
+    const next = new Set(this._access);
+    next.has(userId) ? next.delete(userId) : next.add(userId);
+    this._access = next;
+  }
+
+  async _saveAccess() {
+    this._busy = true;
+    try {
+      await jf(`/api/mcp/global/${this._glob.id}/access`, {
+        method: 'PUT',
+        headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify({ user_ids: [...this._access] }),
+      });
+      announceChange();
+      await this._load();
+    } catch (e) { this._error = e.message; }
+    finally { this._busy = false; }
+  }
+
+  // ── Render ─────────────────────────────────────────────────────────────────
+
+  render() {
+    if (!this._open) return nothing;
+    if (this._error && !this._entry && !this._glob) {
+      return html`
+        

+          ${this._renderHeader()}
+          
${this._error}

+        
`;
+    }
+    if (!this._entry && !this._glob) {
+      return html`
${this._renderHeader()}
+        
 Loading…
`;
+    }
+
+    return html`
+      

+        ${this._renderHeader()}
+        

+          ${this._error ? html`
+            
${this._error}
` : nothing}
+          ${this._renderSummary()}
+          ${this._renderConfig()}
+          ${this._renderAccess()}
+        

+      
`;
+  }
+
+  _renderHeader() {
+    const title = this._entry?.friendly_name || this._glob?.friendly_name || this._name || 'Connector';
+    return html`
+      

+        

+          
+          
${title}

+        

+      
`;
+  }
+
+  _renderSummary() {
+    const e        = this._entry;
+    const isScript = e?.source === 'local_script';
+    const status   = this._status;
+    const desc     = e?.description || this._glob?.description;
+
+    return html`
+      

+        

+          ${!this._noIcon
+            ? html` { this._noIcon = true; }} />`
+            : html`

+                     
`}
+          

+            

+              ${e?.friendly_name || this._glob?.friendly_name || this._name}
+            

+            
${this._name}

+          

+        

+        ${desc ? html`
${desc}
` : nothing}
+        

+          
+            ${this._isGlobal ? 'global' : 'per-user'}
+          
+          ${isScript ? html`
+            
+              runs code on this box
+            ` : nothing}
+          ${e?.auth_kind && e.auth_kind !== 'none' ? html`
+            ${e.auth_kind}` : nothing}
+          ${status === 'active' ? html`
+            active` : nothing}
+          ${status === 'pending' ? html`
+            needs fixing` : nothing}
+          ${status === 'needs_login' ? html`
+            needs sign-in` : nothing}
+        

+        ${this._isGlobal ? html`
+          

+            Runs once for the household, on the host. Nobody reaches it until they are granted access.
+          
` : nothing}
+      
`;
+  }
+
+  _renderConfig() {
+    const e = this._entry;
+
+    // A granted global we have no catalog row for: nothing here is ours to configure.
+    if (!e) {
+      return html`
+        

+          

+            
This connector is managed for you.

+            
It is enabled by an admin and granted to you — there is nothing to configure.

+          

+        
`;
+    }
+
+    const active    = this._isGlobal ? !!this._glob : !!this._act;
+    const canManage = this._isGlobal ? this._isAdmin : true;
+    const hasVerify = !!e.verify_command;
+    const oauth     = e.auth_kind === 'oauth';
+
+    if (this._isGlobal && !this._isAdmin) return nothing;
+
+    // OAuth is a per-user, interactive flow — a browser consent, not a form of
+    // typed credentials — so it gets its own panel instead of the api_key/env body.
+    if (oauth && !this._isGlobal) {
+      return html`
+        

+          

+            
Sign in

+          

+          ${this._renderOauth()}
+        
`;
+    }
+
+    return html`
+      

+        

+          

+            ${active ? 'Configuration' : 'Set up'}
+          

+        

+
+        ${active ? html`
+          

+            ${this._isGlobal
+              ? 'Already enabled. Re-submitting replaces the stored credentials.'
+              : 'Already active. Re-submitting replaces the stored credentials.'}
+          
` : nothing}
+
+        ${e.auth_kind === 'api_key' ? html`
+          

+            
+             { this._form = { ...this._form, api_key: ev.target.value }; }} />
+          
` : nothing}
+
+        ${this._renderEnvFields()}
+        ${this._renderVerifyBox()}
+
+        

+          ${hasVerify && canManage ? html`
+            ` : nothing}
+
+          ${this._isGlobal
+            ? html`
+              
+              ${this._glob ? html`
+                ` : nothing}`
+            : html`
+              
+              ${this._act ? html`
+                ` : nothing}`}
+        

+      
`;
+  }
+
+  _renderOauth() {
+    const provider = this._entry?.oauth_provider || 'provider';
+    const label    = provider.charAt(0).toUpperCase() + provider.slice(1);
+    const active   = this._act && this._act.auth_state === 'ready';
+    const pending  = this._act && this._act.auth_state === 'pending';
+    const scopes   = parseJson(this._entry?.oauth_scopes_json, []);
+
+    return html`
+      

+        Signs in with ${label}. You approve access in a browser tab, then paste back the
+        code the page shows you — nothing is stored on this box until you do.
+      

+
+      ${scopes.length ? html`
+        

+          
It will request access to:

+          
    ${scopes.map(s => html`
  • ${s}
    • `)}

+        
` : nothing}
+
+      ${active ? html`
+        

+          Signed in and active.
+        
` : nothing}
+
+      ${!this._oauth ? html`
+        

+          
+          ${this._act ? html`
+            ` : nothing}
+        
`
+      : html`
+        

+          

+            A tab opened for ${label}. Approve access there.
+            
Re-open the sign-in page

+          

+          

+            Paste the code the page gave you:
+          

+           { this._oauth = { ...this._oauth, code: ev.target.value }; }} />
+          

+            
+            
+          

+        
`}
+    `;
+  }
+
+  _renderEnvFields() {
+    if (!this._schema.length) return nothing;
+    return this._schema.map(f => html`
+      

+        
+         this._patchEnv(f.name, ev.target.value)} />
+        ${f.description ? html`
${f.description}
` : nothing}
+      
`);
+  }
+
+  _renderVerifyBox() {
+    const t = this._test;
+    if (t === null) return nothing;
+    if (t === 'running') {
+      return html`

+        Testing credentials…
`;
+    }
+    if (t.skipped) {
+      return html`

+        ${t.message || 'No verification step for this connector.'}
`;
+    }
+    return html`
+      

+        
+        ${t.ok ? 'OK' : 'Failed'} — ${t.message}
+        ${t.details ? html`
+          
${JSON.stringify(t.details, null, 2)}
` : nothing}
+      
`;
+  }
+
+  /// Who may use this global connector. Only meaningful once it is enabled — there
+  /// is no instance to grant access to before that.
+  _renderAccess() {
+    if (!this._isGlobal || !this._isAdmin || !this._glob) return nothing;
+    const users = this._users ?? [];
+    return html`
+      

+        

+          
Who can use it

+        

+        

+          Ticking a box grants this connector's tools to that person's agent. Saving replaces the whole list.
+        

+        ${users.length === 0
+          ? html`
No users.
`
+          : html`
+            

+              ${users.map(u => html`
+                

+                   this._toggleAccess(u.id)} />
+                  
+                
`)}
+            
`}
+        
+      
`;
+  }
+}
diff --git a/web/components/connectors.js b/web/components/connectors.js
index 226ef87..505cb30 100644
--- a/web/components/connectors.js
+++ b/web/components/connectors.js
@@ -1,28 +1,26 @@
 import { html, nothing } from 'lit';
 import { LightElement } from '../lib/base.js';
+import { connectorIconUrl, statusOf, STATUS_LABEL } from './shared/connector-common.js';
 
 // Connectors (MCP) — blueprint §7/§14/§15.
 //
-// One question: **what is running, and what can I add?** This is the runtime view —
-// literally `UserMcpView` (global ∪ per-user) plus the actions that create those
-// instances. What this box *offers* is a different question, answered by the
-// Connector Catalog page.
+// **One row per connector**, not one per runtime instance. A catalog entry is a
+// template with two runtimes (§7), and a person thinks in terms of "do I have
+// Gmail?" — not "how many `mcp_user_servers` rows named gmail-ish do I own?". So the
+// old three-section split (Mine / Global / Available) is gone: the same connector
+// used to appear twice, once as a template and once as its instance, and the reader
+// had to join the two by eye. Here each connector appears exactly once, and its
+// state is a chip on the card.
 //
-// The same page serves everyone; the admin just has more verbs. A catalog entry is a
-// template with two runtimes (§7), so "Available" is one list with the verb that fits
-// each row: a `per_user` entry says Activate (anyone), a `global` entry says Enable
-// globally (admin only). Enabling a global is the admin's counterpart to activating a
-// per-user one — which is why they live side by side instead of in an admin dungeon.
+// The card is a link, not a form. Everything that needs typing lives on the
+// connector's own page (`#connector?name=X`) — an activation form has as many
+// fields as the connector declares (EMAIL has a dozen), which a fixed-size dialog
+// could never hold.
 //
-// Reuses the shared `um-*` / bootstrap styling (no page-specific CSS).
+// Reuses the marketplace's card styling (`web/css/connectors.css`).
 
 const ADMIN_ID = 'admin';
 
-function parseJson(s, fallback) {
-  if (!s) return fallback;
-  try { return JSON.parse(s); } catch { return fallback; }
-}
-
 async function jf(url, opts) {
   const res = await fetch(url, opts);
   if (!res.ok) throw new Error(await res.text() || `HTTP ${res.status}`);
@@ -38,15 +36,20 @@ export class ConnectorsPage extends LightElement {
       _me:        { state: true },   // { role_id }
       _available: { state: true },   // { catalog: [...], globals: [...] }
       _activated: { state: true },   // my per-user server rows
-      _users:     { state: true },   // admin: user summaries (for the access modal)
       _error:     { state: true },
-      _modal:     { state: true },
+      _q:         { state: true },
+      _noIcon:    { state: true },   // names whose icon failed to load
+      _providers: { state: true },   // admin: OAuth provider list (modal)
+      _pForm:     { state: true },   // admin: provider being edited, or null
+      _pError:    { state: true },
     };
   }
 
   constructor() {
     super();
     this._open = false;
+    this._q = '';
+    this._noIcon = new Set();
     this._reset();
   }
 
@@ -54,9 +57,10 @@ export class ConnectorsPage extends LightElement {
     this._me = null;
     this._available = null;
     this._activated = null;
-    this._users = null;
     this._error = null;
-    this._modal = null;
+    this._providers = null;
+    this._pForm = null;
+    this._pError = null;
   }
 
   connectedCallback() {
@@ -66,6 +70,9 @@ export class ConnectorsPage extends LightElement {
       this.style.display = this._open ? 'flex' : 'none';
       if (this._open) this._load();
     });
+    // Coming back from a connector's page must show its new state, not the state
+    // captured before the user activated it.
+    window.addEventListener('connectors-changed', () => { if (this._open) this._load(); });
   }
 
   get _isAdmin() { return this._me?.role_id === ADMIN_ID; }
@@ -80,130 +87,147 @@ export class ConnectorsPage extends LightElement {
       ]);
       this._available = available;
       this._activated = activated;
-      // Only the access modal needs the user list, and only an admin opens it.
-      if (this._isAdmin) this._users = await jf('/api/users');
     } catch (e) {
       this._error = e.message;
     }
   }
 
-  _patch(field, value) {
-    this._modal = { ...this._modal, form: { ...this._modal.form, [field]: value } };
+  _go(page, hash) {
+    history.pushState({ page }, '', hash);
+    window.dispatchEvent(new CustomEvent('llm-page-change', { detail: { page } }));
   }
 
-  _closeModal() { this._modal = null; this._error = null; }
-
-  _goCatalog() {
-    history.pushState({ page: 'catalog' }, '', '#catalog');
-    window.dispatchEvent(new CustomEvent('llm-page-change', { detail: { page: 'catalog' } }));
+  _openConnector(name) {
+    this._go('connector', `#connector?name=${encodeURIComponent(name)}`);
   }
 
-  // ── Activate a per-user connector ──────────────────────────────────────────
+  // ── admin: OAuth sign-in providers (§15) ─────────────────────────────────────
 
-  _openActivate(entry) {
-    const schema = parseJson(entry.config_schema_json, []) || [];
-    this._modal = {
-      kind: 'activate',
-      entry,
-      form: { name: entry.name, api_key: '', env: Object.fromEntries(schema.map(k => [k, ''])) },
+  async _openProviders() {
+    this._pError = null;
+    this._pForm = null;
+    try {
+      this._providers = await jf('/api/mcp/providers');
+    } catch (e) { this._pError = e.message; this._providers = []; }
+  }
+
+  _closeProviders() {
+    this._providers = null;
+    this._pForm = null;
+    this._pError = null;
+  }
+
+  _blankProvider() {
+    return { name: '', display_name: '', auth_url: '', token_url: '',
+             client_id: '', client_secret: '', redirect_uri: '', extra_params: '' };
+  }
+
+  /// A Google preset — fills everything but the client_id/secret the admin pastes
+  /// from their Google Cloud console. `prompt=consent` + `access_type=offline` are
+  /// what make Google return a refresh token (§15).
+  _presetGoogle() {
+    this._pError = null;
+    this._pForm = {
+      name:          'google',
+      display_name:  'Google',
+      auth_url:      'https://accounts.google.com/o/oauth2/v2/auth',
+      token_url:     'https://oauth2.googleapis.com/token',
+      client_id:     '',
+      client_secret: '',
+      redirect_uri:  'https://connectors.skaldagent.net/oauth/show.html',
+      extra_params:  '{"access_type":"offline","prompt":"consent"}',
+      _isNew:        true,
     };
   }
 
-  async _activate() {
-    const { entry, form } = this._modal;
-    if (!form.name.trim()) { this._error = 'A name is required.'; return; }
-    const env = {};
-    for (const [k, v] of Object.entries(form.env || {})) if (v !== '') env[k] = v;
+  _editProvider(p) {
+    // The secret never came back from the server; an empty box means "keep it".
+    this._pForm = { ...p, client_secret: '', extra_params: p.extra_params || '', _isNew: false };
+    this._pError = null;
+  }
+
+  _patchProvider(key, value) {
+    this._pForm = { ...this._pForm, [key]: value };
+  }
+
+  async _saveProvider() {
+    const f = this._pForm;
+    if (!f.name.trim() || !f.client_id.trim()) {
+      this._pError = 'Name and client id are required.';
+      return;
+    }
+    if (f._isNew && !f.client_secret.trim()) {
+      this._pError = 'A client secret is required for a new provider.';
+      return;
+    }
+    this._pError = null;
     try {
-      await jf('/api/mcp/activate', {
-        method: 'POST',
-        headers: { 'Content-Type': 'application/json' },
-        body: JSON.stringify({
-          catalog_name: entry.name,
-          name: form.name.trim(),
-          api_key: form.api_key || null,
-          env: Object.keys(env).length ? env : null,
-        }),
+      const { _isNew, has_client_secret, ...body } = f;
+      await jf('/api/mcp/providers', {
+        method: 'POST', headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify(body),
       });
-      this._closeModal();
-      await this._load();
-    } catch (e) { this._error = e.message; }
+      this._pForm = null;
+      this._providers = await jf('/api/mcp/providers');
+    } catch (e) { this._pError = e.message; }
   }
 
-  async _deactivate(row) {
-    if (!confirm(`Deactivate connector "${row.name}"?`)) return;
+  async _deleteProvider(name) {
+    if (!confirm(`Delete the “${name}” sign-in provider?\n\nConnectors that use it will no longer be able to sign in.`)) return;
     try {
-      await jf(`/api/mcp/activated/${row.id}`, { method: 'DELETE' });
-      await this._load();
-    } catch (e) { this._error = e.message; }
+      await jf(`/api/mcp/providers/${encodeURIComponent(name)}`, { method: 'DELETE' });
+      this._providers = await jf('/api/mcp/providers');
+    } catch (e) { this._pError = e.message; }
   }
 
-  // ── Enable a global connector (admin) ──────────────────────────────────────
+  /// The merged view: every connector the caller can see, exactly once, carrying
+  /// whichever runtime rows exist for it.
+  get _rows() {
+    const catalog   = this._available?.catalog ?? [];
+    const globals   = this._available?.globals ?? [];
+    const activated = this._activated ?? [];
 
-  // The entry comes from the row the admin clicked, so there is no catalog picker:
-  // the old dropdown existed only because this action lived on a page that did not
-  // show the catalog.
-  _openEnableGlobal(entry) {
-    this._modal = {
-      kind: 'global',
-      entry,
-      form: { name: entry.name, api_key: '' },
-    };
-  }
+    const rows = catalog.map(e => ({
+      ...e,
+      _act:  activated.find(r => r.catalog_name === e.name) ?? null,
+      _glob: globals.find(g => (g.catalog_name ?? g.name) === e.name) ?? null,
+    }));
 
-  async _enableGlobal() {
-    const { entry, form } = this._modal;
-    try {
-      await jf('/api/mcp/global', {
-        method: 'POST',
-        headers: { 'Content-Type': 'application/json' },
-        body: JSON.stringify({
-          catalog_name: entry.name,
-          name: form.name.trim() || null,
-          api_key: form.api_key || null,
-        }),
+    // A granted global whose catalog row the caller cannot see. `/api/mcp/available`
+    // only returns `global` catalog entries to a catalog manager, so without this the
+    // connector an ordinary user actually uses every day would be missing from their
+    // own list — visible to the admin, invisible to its user.
+    for (const g of globals) {
+      const key = g.catalog_name ?? g.name;
+      if (rows.some(r => r.name === key)) continue;
+      rows.push({
+        name:          key,
+        friendly_name: g.friendly_name,
+        description:   g.description,
+        scope:         'global',
+        source:        'remote',
+        auth_kind:     'none',
+        _act:          null,
+        _glob:         g,
       });
-      this._closeModal();
-      await this._load();
-    } catch (e) { this._error = e.message; }
+    }
+
+    const q = this._q.trim().toLowerCase();
+    return rows
+      .filter(r => !q
+        || r.name.toLowerCase().includes(q)
+        || (r.friendly_name ?? '').toLowerCase().includes(q)
+        || (r.description ?? '').toLowerCase().includes(q))
+      .sort((a, b) => (a.friendly_name || a.name).localeCompare(b.friendly_name || b.name));
   }
 
-  async _deleteGlobal(row) {
-    if (!confirm(`Disable global connector "${row.name}"?\n\nIt stops for everyone who can use it.`)) return;
-    try {
-      await jf(`/api/mcp/global/${row.id}`, { method: 'DELETE' });
-      await this._load();
-    } catch (e) { this._error = e.message; }
-  }
-
-  async _openAccess(server) {
-    this._modal = { kind: 'access', server, selected: new Set() };
-    try {
-      const current = await jf(`/api/mcp/global/${server.id}/access`);
-      // Ignore if the admin already navigated away / opened another modal.
-      if (this._modal?.kind === 'access' && this._modal.server.id === server.id) {
-        this._modal = { ...this._modal, selected: new Set(current || []) };
-      }
-    } catch (e) { this._error = e.message; }
-  }
-
-  _toggleAccess(userId) {
-    const sel = new Set(this._modal.selected);
-    sel.has(userId) ? sel.delete(userId) : sel.add(userId);
-    this._modal = { ...this._modal, selected: sel };
-  }
-
-  async _saveAccess() {
-    const { server, selected } = this._modal;
-    try {
-      await jf(`/api/mcp/global/${server.id}/access`, {
-        method: 'PUT',
-        headers: { 'Content-Type': 'application/json' },
-        body: JSON.stringify({ user_ids: [...selected] }),
-      });
-      this._closeModal();
-      await this._load();
-    } catch (e) { this._error = e.message; }
+  _iconFailed(name) {
+    // Re-render with the placeholder. A synthetic row (a granted global whose
+    // catalog entry the caller cannot read) has no icon path to check up front, so
+    // the 404 is the check.
+    const next = new Set(this._noIcon);
+    next.add(name);
+    this._noIcon = next;
   }
 
   // ── Render ─────────────────────────────────────────────────────────────────
@@ -211,6 +235,7 @@ export class ConnectorsPage extends LightElement {
   render() {
     if (!this._open) return nothing;
     const loading = this._available === null && !this._error;
+    const rows = loading ? [] : this._rows;
 
     return html`
       

@@ -218,232 +243,187 @@ export class ConnectorsPage extends LightElement {
           
Connectors

           

             ${this._isAdmin ? html`
-              
+              
+              ` : nothing}
           

         

 
-        ${this._error && !this._modal ? html`
+        ${this._error ? html`
           
${this._error}
` : nothing}
 
-        ${loading ? html`
 Loading…
` : html`
-          

-            ${this._renderMine()}
-            ${this._renderGlobals()}
-            ${this._renderAvailable()}
-          
`}
-      
-      ${this._renderModal()}`;
-  }
-
-  _section(title, icon, right, body) {
-    return html`
-      

-        

-          
${title}

-          
${right ?? nothing}

-        

-        ${body}
+        ${loading
+          ? html`
 Loading…
`
+          : html`
+            

+              

+                

+                  
+                   { this._q = e.target.value; }} />
+                

+              

+              ${rows.length === 0 ? this._renderEmpty() : html`
+                
${rows.map(r => this._renderCard(r))}
`}
+            
`}
+        ${this._providers !== null ? this._renderProvidersModal() : nothing}
       
`;
   }
 
-  _renderMine() {
-    const rows = this._activated ?? [];
-    return this._section('My connectors', 'bi-check2-circle', nothing,
-      rows.length === 0
-        ? html`

-                 
No per-user connectors activated.
`
-        : html`
-          
-            
-            
-              ${rows.map(r => html`
-                
-                  
-                  
-                  
-                  
-                `)}
-            
-          
NameTypeFrom catalog
${r.name}${r.source === 'local_script' ? 'local script' : 'remote'}${r.catalog_name ? html`${r.catalog_name}` : html`—`}

-                    
-                  
`);
-  }
-
-  _renderGlobals() {
-    const rows = this._available?.globals ?? [];
-    if (rows.length === 0 && !this._isAdmin) return nothing;
-    return this._section('Global connectors', 'bi-globe', nothing,
-      rows.length === 0
-        ? html`

-                 
None enabled. Enable one from Available below.
`
-        : html`
-          ${this._isAdmin ? html`
-            

-              Shared by the household. You see every one so you can manage it —
-              yours marks the ones granted to you.
-            
` : nothing}
-          
-            
-            
-              ${rows.map(g => html`
-                
-                  
-                  
-                  
-                  
-                `)}
-            
-          
NameTransportStatus

-                    ${g.friendly_name || g.name}
-                    ${this._isAdmin && g.can_use ? html`
-                      yours` : nothing}
-                    ${g.description ? html`
-                      
${g.description}
` : nothing}
-                  		${g.transport}${g.enabled
-                    ? html`on`
-                    : html`off`}

-                    ${this._isAdmin ? html`
-                      
-                      
-                    ` : nothing}
-                  
`);
-  }
-
-  _renderAvailable() {
-    const entries = this._available?.catalog ?? [];
-    const enabledGlobals = new Set((this._available?.globals ?? []).map(g => g.catalog_name ?? g.name));
-    const activatedNames = new Set((this._activated ?? []).map(r => r.catalog_name));
-
-    const right = this._isAdmin ? html`
-      ` : nothing;
-
-    if (entries.length === 0) {
-      return this._section('Available', 'bi-plus-square', right, html`
-        

-          
${this._isAdmin ? 'The catalog is empty.' : 'Nothing available to you yet.'}

-          ${this._isAdmin ? html`
-            
Add connectors to the catalog first.
` : nothing}
-        
`);
-    }
-
-    return this._section('Available', 'bi-plus-square', right, html`
-      
-        
-        
-          ${entries.map(e => {
-            const isGlobal = e.scope === 'global';
-            const already = isGlobal ? enabledGlobals.has(e.name) : activatedNames.has(e.name);
-            return html`
-              
-                
-                
-                
-                
-              `;
-          })}
-        
-      
ConnectorScopeAuth
${e.friendly_name || e.name}
-                  ${e.description ? html`
-                    
${e.description}
` : nothing}		
-                  ${isGlobal ? 'global' : 'per-user'}${e.auth_kind}

-                  ${already
-                    ? html`${isGlobal ? 'enabled' : 'active'}`
-                    : isGlobal
-                      ? html``
-                      : html``}
-                
`);
-  }
-
-  // ── Modals ─────────────────────────────────────────────────────────────────
-
-  _modalShell(title, icon, body, onSave, saveLabel) {
+  _renderProvidersModal() {
     return html`
-      
 { if (e.target.classList.contains('um-modal-overlay')) this._closeModal(); }}>
-        

-          

-            ${title}
-            
+      
 { if (e.target === e.currentTarget) this._closeProviders(); }}>
+        

+          

+            
Sign-in providers

+            
           

-          

-            ${this._error ? html`
${this._error}
` : nothing}
-            ${body}
-          

-          

-            
-            
+          

+            OAuth apps that per-user connectors sign in through. One app (e.g. Google) covers all of
+            its services. The client secret is stored on this box and never shown again.
           

+          ${this._pError ? html`
+            
${this._pError}
` : nothing}
+          ${this._pForm ? this._renderProviderForm() : this._renderProviderList()}
         

       
`;
   }
 
-  _field(label, value, oninput, opts = {}) {
-    return html`

-      
-      
-    
`;
+  _renderProviderList() {
+    const list = this._providers ?? [];
+    return html`
+      ${list.length === 0 ? html`
+        

+          
No sign-in providers yet.
` : html`
+        

+          ${list.map(p => html`
+            

+              

+                
${p.display_name || p.name}
+                  ${p.name}

+                

+                  ${p.has_client_secret
+                    ? html` secret set`
+                    : html` no secret`}
+                  · ${p.client_id || '(no client id)'}
+                

+              

+              

+                
+                
+              

+            
`)}
+        
`}
+      

+        
+        
+      
`;
   }
 
-  _renderModal() {
-    if (!this._modal) return nothing;
-    const m = this._modal;
+  _renderProviderForm() {
+    const f = this._pForm;
+    const field = (key, label, opts = {}) => html`
+      

+        
+         this._patchProvider(key, e.target.value)} />
+        ${opts.help ? html`
${opts.help}
` : nothing}
+      
`;
+    return html`
+      ${field('name', 'Provider id', { req: true, mono: true, ph: 'google',
+        help: 'The slug a connector references (must match the manifest\'s auth.provider).' })}
+      ${field('display_name', 'Display name', { ph: 'Google' })}
+      ${field('client_id', 'Client id', { req: true, mono: true })}
+      ${field('client_secret', 'Client secret', { secret: true, mono: true,
+        help: f._isNew ? 'Required.' : 'Leave blank to keep the stored secret.' })}
+      ${field('auth_url', 'Authorization URL', { mono: true, ph: 'https://accounts.google.com/o/oauth2/v2/auth' })}
+      ${field('token_url', 'Token URL', { mono: true, ph: 'https://oauth2.googleapis.com/token' })}
+      ${field('redirect_uri', 'Redirect URI', { mono: true,
+        help: 'The copy-paste page. Must be registered as an authorized redirect in the provider\'s console.' })}
+      ${field('extra_params', 'Extra params (JSON)', { mono: true, ph: '{"access_type":"offline","prompt":"consent"}',
+        help: 'Merged into the consent URL. Google needs these two to return a refresh token.' })}
+      

+        
+        
+      
`;
+  }
 
-    if (m.kind === 'activate') {
-      const f = m.form;
-      const schema = parseJson(m.entry.config_schema_json, []) || [];
-      return this._modalShell(`Activate ${m.entry.friendly_name || m.entry.name}`, 'bi-plug', html`
-        ${this._field('Name', f.name, e => this._patch('name', e.target.value), { hint: 'unique for you', mono: true })}
-        ${m.entry.auth_kind === 'api_key' ? this._field('API key', f.api_key, e => this._patch('api_key', e.target.value), { type: 'password', mono: true }) : nothing}
-        ${m.entry.auth_kind === 'oauth' ? html`
-          

-            This connector needs an interactive login,
-            which is not wired up yet — it will activate but cannot authenticate.
-          
` : nothing}
-        ${schema.map(k => html`

-          
-           this._patch('env', { ...f.env, [k]: e.target.value })} />
-        
`)}
-      `, () => this._activate(), 'Activate');
+  _renderEmpty() {
+    if (this._q.trim()) {
+      return html`

+        
No connector matches “${this._q}”.
`;
     }
+    return html`
+      

+        
${this._isAdmin ? 'No connectors installed yet.' : 'Nothing available to you yet.'}

+        ${this._isAdmin
+          ? html`
Install one from the Marketplace to get started.
`
+          : html`
Ask an admin to make one available.
`}
+      
`;
+  }
 
-    if (m.kind === 'global') {
-      const f = m.form;
-      return this._modalShell(`Enable ${m.entry.friendly_name || m.entry.name} globally`, 'bi-globe', html`
-        

-          Runs once for the household on the host. Nobody reaches it until you grant access.
+  _renderCard(r) {
+    const status   = statusOf(r);
+    const isGlobal = r.scope === 'global';
+    const isScript = r.source === 'local_script';
+    const showIcon = !this._noIcon.has(r.name);
+
+    return html`
+      
 this._openConnector(r.name)}
+        @keydown=${(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); this._openConnector(r.name); } }}>
+        

+          ${showIcon
+            ? html` this._iconFailed(r.name)} />`
+            : html`
`}
+          

+            
${r.friendly_name || r.name}

+            
${r.name}

+          

+          
+            ${STATUS_LABEL[status].text}
+          
         

-        ${this._field('Name', f.name, e => this._patch('name', e.target.value), { hint: 'runtime name', mono: true })}
-        ${m.entry.auth_kind === 'api_key'
-          ? this._field('API key', f.api_key, e => this._patch('api_key', e.target.value), { type: 'password', mono: true })
-          : nothing}
-      `, () => this._enableGlobal(), 'Enable');
-    }
 
-    if (m.kind === 'access') {
-      const users = this._users ?? [];
-      return this._modalShell(`Access — ${m.server.name}`, 'bi-people', html`
-        
Select who may use this global connector. This replaces the current list.

-        ${users.map(u => html`

-           this._toggleAccess(u.id)} />
-          
-        
`)}
-      `, () => this._saveAccess(), 'Save access');
-    }
+        ${r.description ? html`
${r.description}
` : nothing}
 
-    return nothing;
+        

+          
+            ${isGlobal ? 'global' : 'per-user'}
+          
+          ${isScript ? html`
+            
+              local script
+            ` : nothing}
+          ${r.auth_kind && r.auth_kind !== 'none' ? html`
+            ${r.auth_kind}` : nothing}
+        

+      
`;
   }
 }
diff --git a/web/components/marketplace.js b/web/components/marketplace.js
index 76cdc36..ddfb7bd 100644
--- a/web/components/marketplace.js
+++ b/web/components/marketplace.js
@@ -91,7 +91,7 @@ export class MarketplacePage extends LightElement {
 
   async _install(card) {
     const warn = card.source === 'local_script'
-      ? `\n\nThis puts code on this box:\n  • ${card.file_count} file(s), each verified against its SHA-256\n  • installed into ./scripts/${card.id}/`
+      ? `\n\nThis puts code on this box:\n  • ${card.file_count} file(s), each verified against its SHA-256\n  • installed into ./connectors/${card.id}/`
       : '';
     if (!confirm(`Install "${card.name}" into the catalog?${warn}\n\nInstalling does not activate it.`)) return;
     this._installing = card.id;
diff --git a/web/components/shared/connector-common.js b/web/components/shared/connector-common.js
new file mode 100644
index 0000000..6701c09
--- /dev/null
+++ b/web/components/shared/connector-common.js
@@ -0,0 +1,88 @@
+// Shared vocabulary for the Connectors list and a connector's own page.
+//
+// Both surfaces have to answer "what state is this connector in?" and both draw the
+// same env/secret form. Deriving that twice is how the two drift, so the derivation
+// lives here and each page only decides layout.
+
+/// The icon of an **installed** connector, off the box's own `connectors/` folder —
+/// not the marketplace proxy, which is admin-only and dies with the feed.
+export function connectorIconUrl(name, size = 'sm') {
+  return `/api/mcp/catalog/${encodeURIComponent(name)}/icon?size=${size}`;
+}
+
+/// How each status reads on a chip. `tone` maps to the `connector-chip--*` accents
+/// in `web/css/connectors.css`.
+export const STATUS_LABEL = {
+  active:      { text: 'active',        tone: 'ok' },
+  pending:     { text: 'needs fix',     tone: 'script' },
+  needs_login: { text: 'needs sign-in', tone: 'script' },
+  enabled:     { text: 'enabled',       tone: 'scope' },
+  off:         { text: 'off',           tone: '' },
+  available:   { text: 'available',     tone: '' },
+};
+
+/// The one place that decides what a connector's state *is*, from whichever runtime
+/// rows exist for it.
+///
+/// A per-user activation whose credentials failed verification is `pending`, not
+/// `active`: the row exists but is deliberately held out of `all_startable`, and
+/// calling that "active" would be a lie the user acts on.
+///
+/// `enabled` vs `active` for a global is the §7 distinction between *running* and
+/// *reachable by me*: an admin can enable a connector for someone else and never
+/// grant it to themselves, and their own list must not claim they have it.
+export function statusOf(row) {
+  if (row._act) {
+    if (row._act.auth_state !== 'pending') return 'active';
+    // An OAuth connector sitting at `pending` is waiting for its interactive
+    // sign-in, not for a failed credential to be fixed — a different ask.
+    return row._act.oauth_provider ? 'needs_login' : 'pending';
+  }
+  if (row._glob) {
+    if (!row._glob.enabled) return 'off';
+    return row._glob.can_use ? 'active' : 'enabled';
+  }
+  return 'available';
+}
+
+/// Normalizes a catalog entry's `config_schema_json` into form-field descriptors,
+/// whether the feed shipped the object-array form or the legacy bare-name list.
+export function normalizeSchema(raw) {
+  if (!Array.isArray(raw)) return [];
+  return raw.map(e => {
+    if (typeof e === 'string') {
+      return { name: e, label: e, description: '', required: false, secret: false, example: '', default: '' };
+    }
+    return {
+      name:        e.name || '',
+      label:       e.label || e.name || '',
+      description: e.description || '',
+      required:    !!e.required,
+      secret:      !!e.secret,
+      example:     e.example || '',
+      default:     e.default || '',
+    };
+  });
+}
+
+export function parseJson(s, fallback) {
+  if (!s) return fallback;
+  try { return JSON.parse(s); } catch { return fallback; }
+}
+
+/// Every schema field seeded with its `default` (empty string when none).
+export function seedEnv(schema) {
+  return Object.fromEntries(schema.map(e => [e.name, e.default || '']));
+}
+
+export async function jf(url, opts) {
+  const res = await fetch(url, opts);
+  if (!res.ok) throw new Error(await res.text() || `HTTP ${res.status}`);
+  const ct = res.headers.get('content-type') || '';
+  return ct.includes('application/json') ? res.json() : null;
+}
+
+/// Tells the Connectors list its cached state is stale.
+export function announceChange() {
+  window.dispatchEvent(new CustomEvent('connectors-changed'));
+}
diff --git a/web/components/sidebar.js b/web/components/sidebar.js
index 9aa92d3..92a4a68 100644
--- a/web/components/sidebar.js
+++ b/web/components/sidebar.js
@@ -119,7 +119,8 @@ export class AppSidebar extends LightElement {
     // Segment ends at the first `/` (e.g. `#session/123`) or `?` (e.g. `#file_viewer?path=...`).
     const match = hash.match(/^([^/?]+)/);
     const segment = match ? match[1] : '';
-    return ['inbox', 'tasks', 'projects', 'models', 'providers', 'approval', 'agents', 'users', 'roles', 'connectors', 'catalog', 'marketplace', 'profile', 'config', 'llm-requests', 'session', 'tic', 'file_viewer'].includes(segment) ? segment : 'home';
+    // `connector` (singular) is the per-connector detail page, `connectors` the list.
+    return ['inbox', 'tasks', 'projects', 'models', 'providers', 'approval', 'agents', 'users', 'roles', 'connectors', 'connector', 'catalog', 'marketplace', 'profile', 'config', 'llm-requests', 'session', 'tic', 'file_viewer'].includes(segment) ? segment : 'home';
   }
 
   _tasksSectionFromHash() {
@@ -299,7 +300,7 @@ export class AppSidebar extends LightElement {
           
           Roles
         
-         this._togglePage('connectors', e)}>
           
           Connectors
diff --git a/web/css/page-shell.css b/web/css/page-shell.css
index d1e34a6..ae5ee47 100644
--- a/web/css/page-shell.css
+++ b/web/css/page-shell.css
@@ -74,6 +74,7 @@ file-viewer-page {
 users-page,
 roles-page,
 connectors-page,
+connector-detail-page,
 marketplace-page,
 catalog-page,
 profile-page {
diff --git a/web/index.html b/web/index.html
index 8b20546..2cda2fb 100644
--- a/web/index.html
+++ b/web/index.html
@@ -94,6 +94,7 @@
       
       
       
+