Compare commits
2
Commits
c0a779b79e
...
fb6f8ef195
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
fb6f8ef195 | ||
|
|
548871fc72 |
@@ -172,13 +172,28 @@ pub const PERSISTED_REQUEST_ID: i64 = 0;
|
||||
// ── Session bypass ────────────────────────────────────────────────────────────
|
||||
|
||||
/// What a session bypass entry applies to.
|
||||
///
|
||||
/// [`Tool`](Self::Tool) is the **default** scope of the "15 min" / "Session"
|
||||
/// buttons on an approval card, and the only one narrow enough to be safe to
|
||||
/// pick on the user's behalf: a human answering a card has read *that* call,
|
||||
/// and nothing else. The wider scopes stay reachable through the REST
|
||||
/// `bypass_scope` field, where choosing one is a deliberate act.
|
||||
pub enum BypassScope {
|
||||
/// Covers every tool regardless of category.
|
||||
All,
|
||||
/// Covers exactly one tool, matched on its full name
|
||||
/// (`mcp__gmail__send_message`, `write_file`, …).
|
||||
Tool(String),
|
||||
/// Covers only tools of the given registered category.
|
||||
Category(ToolCategory),
|
||||
/// Covers only tools belonging to the named MCP server
|
||||
/// (matched by the `mcp__<server>__` prefix in the tool name).
|
||||
///
|
||||
/// **A connector is not a permission unit**: its read tools and its write
|
||||
/// tools live under one name, so this scope reads "trust everything Gmail
|
||||
/// can do" — including sending mail — from a click on a card that asked
|
||||
/// about labelling a message. Never auto-detect it; require the caller to
|
||||
/// name it.
|
||||
McpServer(String),
|
||||
}
|
||||
|
||||
@@ -715,6 +730,22 @@ impl ApprovalManager {
|
||||
info!(session_id, secs = duration.as_secs(), "approval: bypass active (timed)");
|
||||
}
|
||||
|
||||
/// Bypasses approval prompts for one tool, matched on its full name.
|
||||
/// `duration` is `None` for an indefinite (session-scoped) bypass.
|
||||
pub async fn bypass_session_for_tool(
|
||||
&self,
|
||||
session_id: i64,
|
||||
tool: String,
|
||||
duration: Option<Duration>,
|
||||
) {
|
||||
let expires_at = duration.map(|d| Instant::now() + d);
|
||||
self.session_bypasses.lock().await
|
||||
.entry(session_id)
|
||||
.or_default()
|
||||
.push(ApprovalBypass { scope: BypassScope::Tool(tool.clone()), expires_at });
|
||||
info!(session_id, tool, secs = duration.map(|d| d.as_secs()), "approval: bypass active (tool)");
|
||||
}
|
||||
|
||||
/// Bypasses approval prompts for a specific tool `category`.
|
||||
/// `duration` is `None` for an indefinite (session-scoped) bypass.
|
||||
pub async fn bypass_session_for_category(
|
||||
@@ -892,14 +923,21 @@ impl ApprovalManager {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Approve + register a session bypass so future tool calls of the same
|
||||
/// category / MCP server are auto-approved.
|
||||
/// Approve + register a session bypass so future calls of the **same tool**
|
||||
/// are auto-approved.
|
||||
///
|
||||
/// - `bypass_secs = Some(n)`: bypass lasts `n` seconds (0 is treated as indefinite)
|
||||
/// - `bypass_secs = None`: bypass lasts until the session ends
|
||||
///
|
||||
/// Scope is auto-detected from the pending request's tool metadata,
|
||||
/// mirroring the web-inbox logic in `src/frontend/api/inbox.rs`.
|
||||
/// The scope is always [`BypassScope::Tool`] and is deliberately **not**
|
||||
/// inferred from the tool's category or MCP server. It used to be: a click
|
||||
/// on a Gmail card registered a bypass over the whole connector, so
|
||||
/// approving `mcp__gmail__modify_message` silently un-gated
|
||||
/// `mcp__gmail__send_message` — an explicit `require` rule on it and all —
|
||||
/// and the only trace was a log line. A human answering a card has read one
|
||||
/// call; that call is the widest thing their click may authorise. The
|
||||
/// broader scopes remain available to a caller that names one (the REST
|
||||
/// `bypass_scope` field in `src/frontend/api/inbox.rs`).
|
||||
pub async fn approve_with_bypass(&self, request_id: i64, bypass_secs: Option<u64>) {
|
||||
let info = self.get_pending(request_id).await;
|
||||
self.approve(request_id).await;
|
||||
@@ -907,16 +945,7 @@ impl ApprovalManager {
|
||||
let duration = bypass_secs
|
||||
.filter(|&s| s > 0)
|
||||
.map(Duration::from_secs);
|
||||
if let Some(cat) = info.tool_category {
|
||||
self.bypass_session_for_category(info.session_id, cat, duration).await;
|
||||
} else if let Some(srv) = info.mcp_server {
|
||||
self.bypass_session_for_mcp(info.session_id, srv, duration).await;
|
||||
} else {
|
||||
match duration {
|
||||
Some(d) => self.bypass_session_for(info.session_id, d).await,
|
||||
None => self.bypass_session(info.session_id).await,
|
||||
}
|
||||
}
|
||||
self.bypass_session_for_tool(info.session_id, info.tool_name, duration).await;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1022,6 +1051,7 @@ pub(crate) fn pattern_matches(pattern: &str, tool_name: &str) -> bool {
|
||||
fn bypass_matches(bypass: &ApprovalBypass, category: Option<ToolCategory>, tool_name: &str) -> bool {
|
||||
match &bypass.scope {
|
||||
BypassScope::All => true,
|
||||
BypassScope::Tool(name) => name == tool_name,
|
||||
BypassScope::Category(bc) => category.map_or(false, |tc| tc == *bc),
|
||||
BypassScope::McpServer(server) => {
|
||||
mcp_server_from_tool_name(tool_name).map_or(false, |s| s == *server)
|
||||
@@ -1112,6 +1142,76 @@ mod tests {
|
||||
assert!(pattern_matches("data/*", "data/x"));
|
||||
}
|
||||
|
||||
/// A bypass answered from a card covers **that tool only**.
|
||||
///
|
||||
/// The regression: approving `mcp__gmail__modify_message` with "15 min" used to
|
||||
/// register a bypass over the whole `gmail` connector, so the very next
|
||||
/// `mcp__gmail__send_message` executed without a prompt — through an explicit
|
||||
/// `require` rule written for it — and the only evidence was a log line.
|
||||
#[tokio::test]
|
||||
async fn a_tool_bypass_does_not_cover_its_connector() {
|
||||
use super::{ApprovalManager, GateResult};
|
||||
use serde_json::json;
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::broadcast;
|
||||
|
||||
let path = std::env::temp_dir().join(format!("skald_bypass_test_{}.db", std::process::id()));
|
||||
let path_str = path.to_string_lossy().to_string();
|
||||
let _ = std::fs::remove_file(&path);
|
||||
let pool = crate::db::init_system_pool(&path_str).await.expect("init_system_pool");
|
||||
let db = Arc::new(pool);
|
||||
|
||||
sqlx::query("INSERT INTO tool_permission_groups (id, name) VALUES ('default', 'Default')")
|
||||
.execute(db.as_ref()).await.unwrap();
|
||||
for (tool, action) in [
|
||||
("mcp__gmail__modify_message", "require"),
|
||||
("mcp__gmail__send_message", "require"),
|
||||
] {
|
||||
sqlx::query(
|
||||
"INSERT INTO approval_rules (tool_pattern, action, priority, group_id)
|
||||
VALUES (?, ?, 0, 'default')",
|
||||
)
|
||||
.bind(tool).bind(action).execute(db.as_ref()).await.unwrap();
|
||||
}
|
||||
|
||||
let (tx, _rx) = broadcast::channel(16);
|
||||
let mgr = ApprovalManager::new(Arc::clone(&db), tx);
|
||||
mgr.seed_default_catch_all().await.unwrap();
|
||||
|
||||
let decide = |tool: &'static str| {
|
||||
let mgr = &mgr;
|
||||
async move {
|
||||
mgr.check(1, None, "assistant", "web", tool, &json!({}), Some("default")).await
|
||||
}
|
||||
};
|
||||
|
||||
// Both gated to begin with.
|
||||
assert!(matches!(decide("mcp__gmail__modify_message").await, GateResult::Require));
|
||||
assert!(matches!(decide("mcp__gmail__send_message").await, GateResult::Require));
|
||||
|
||||
// The human approves ONE call with a bypass.
|
||||
mgr.bypass_session_for_tool(1, "mcp__gmail__modify_message".into(), None).await;
|
||||
|
||||
// It covers that tool…
|
||||
assert!(matches!(decide("mcp__gmail__modify_message").await, GateResult::Allow));
|
||||
// …and nothing else on the same connector.
|
||||
assert!(matches!(decide("mcp__gmail__send_message").await, GateResult::Require));
|
||||
// Nor another session's calls (the map is keyed by conversation).
|
||||
let other = mgr
|
||||
.check(2, None, "assistant", "web", "mcp__gmail__modify_message", &json!({}), Some("default"))
|
||||
.await;
|
||||
assert!(matches!(other, GateResult::Require));
|
||||
|
||||
// The connector-wide scope still exists for a caller that names it.
|
||||
mgr.bypass_session_for_mcp(1, "gmail".into(), None).await;
|
||||
assert!(matches!(decide("mcp__gmail__send_message").await, GateResult::Allow));
|
||||
|
||||
db.close().await;
|
||||
for suffix in ["", "-wal", "-shm"] {
|
||||
let _ = std::fs::remove_file(format!("{path_str}{suffix}"));
|
||||
}
|
||||
}
|
||||
|
||||
// End-to-end: run the real startup pipeline (migrate → seed) against a temp SQLite
|
||||
// DB pre-loaded with legacy rules, then assert the gate decisions through `check()`.
|
||||
#[tokio::test]
|
||||
|
||||
@@ -16,7 +16,13 @@
|
||||
# (~270 MB, only for `pip install` of a package with no wheel) and `pandoc`
|
||||
# (~216 MB, niche) are big *and* self-recoverable, so they stay on demand.
|
||||
|
||||
FROM debian:bookworm-slim
|
||||
# Trixie (Debian 13), not bookworm, for python3 >= 3.12: connectors that pull a
|
||||
# modern PyPI package are increasingly gated on it (mcp-server-linkedin declares
|
||||
# `requires-python >=3.12,<3.15`), and `install::ensure_installed` runs the deps
|
||||
# install as a plain `python3 -m pip` — so the system interpreter is the floor
|
||||
# every python connector builds against. Trixie ships 3.13. Note this also moves
|
||||
# node 18 -> 20 and tesseract 5.3 -> 5.5.
|
||||
FROM debian:trixie-slim
|
||||
|
||||
ENV DEBIAN_FRONTEND=noninteractive
|
||||
|
||||
@@ -57,6 +63,39 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
|
||||
tesseract-ocr \
|
||||
tesseract-ocr-ita \
|
||||
tesseract-ocr-fra \
|
||||
# Shared libraries a headless Chromium links against, for connectors that
|
||||
# drive a real browser (the LinkedIn connector via patchright). Only the
|
||||
# libs: the browser *binary* is NOT baked in — the connector downloads its
|
||||
# own pinned build into `PLAYWRIGHT_BROWSERS_PATH` under its connector dir,
|
||||
# where it is durable across container recreates. That split is deliberate:
|
||||
# a pip/npm install can fetch a binary, but it cannot supply system libs, so
|
||||
# these are the part that is genuinely not self-recoverable. Cheap here —
|
||||
# most are already pulled in transitively by ffmpeg/imagemagick/tesseract.
|
||||
# The list is patchright's own `nativeDeps` table for debian13; the `t64`
|
||||
# suffixes are Debian 13's 64-bit time_t transition and are NOT optional.
|
||||
libasound2t64 \
|
||||
libatk-bridge2.0-0t64 \
|
||||
libatk1.0-0t64 \
|
||||
libatspi2.0-0t64 \
|
||||
libcairo2 \
|
||||
libcups2t64 \
|
||||
libdbus-1-3 \
|
||||
libdrm2 \
|
||||
libgbm1 \
|
||||
libglib2.0-0t64 \
|
||||
libnspr4 \
|
||||
libnss3 \
|
||||
libpango-1.0-0 \
|
||||
libx11-6 \
|
||||
libxcb1 \
|
||||
libxcomposite1 \
|
||||
libxdamage1 \
|
||||
libxext6 \
|
||||
libxfixes3 \
|
||||
libxkbcommon0 \
|
||||
libxrandr2 \
|
||||
fonts-liberation \
|
||||
fonts-noto-color-emoji \
|
||||
&& rm -rf /var/lib/apt/lists/*
|
||||
|
||||
# The container runs as the host process's uid:gid (blueprint §6 UID coherence), so
|
||||
|
||||
@@ -37,9 +37,11 @@ use crate::tools::fs as fs_tools;
|
||||
/// suffix is the image cache-buster: [`ContainerManager::ensure_image`] rebuilds only
|
||||
/// when the tag is absent, so **bump it whenever the [`Dockerfile`] changes** (`v2`
|
||||
/// added `sudo` + a NOPASSWD sudoers for the non-root container user; `v3` added
|
||||
/// `unzip` + `ffmpeg`). Old tags linger as orphaned images (harmless), but existing
|
||||
/// containers still *run* one — which is why [`reusable`] also compares the image.
|
||||
const IMAGE_TAG: &str = "skald-runtime:v3";
|
||||
/// `unzip` + `ffmpeg`; `v4` moved the base to Debian 13 for python3 >= 3.12 and
|
||||
/// added the headless-Chromium shared libs). Old tags linger as orphaned images
|
||||
/// (harmless), but existing containers still *run* one — which is why [`reusable`]
|
||||
/// also compares the image.
|
||||
const IMAGE_TAG: &str = "skald-runtime:v4";
|
||||
|
||||
/// The embedded Dockerfile — the source of truth, so the image can be built with
|
||||
/// no files shipped alongside the binary (binary-first).
|
||||
|
||||
+24
-19
@@ -145,7 +145,12 @@ pub struct ApproveBody {
|
||||
/// Seconds for the bypass duration. `0` means indefinite (session-scoped).
|
||||
/// Absent means no bypass.
|
||||
pub bypass_secs: Option<u64>,
|
||||
/// `"category"` | `"mcp_server"` | `"all"`. Defaults to auto-detect from tool info.
|
||||
/// `"tool"` | `"category"` | `"mcp_server"` | `"all"`. Defaults to `"tool"`.
|
||||
///
|
||||
/// Deliberately **not** auto-detected from the tool's metadata any more: a
|
||||
/// click on an approval card authorises the call the human just read, and
|
||||
/// widening that to the tool's category or its whole MCP connector is a
|
||||
/// decision only an explicit value may make (see `approve_with_bypass`).
|
||||
pub bypass_scope: Option<String>,
|
||||
}
|
||||
|
||||
@@ -171,28 +176,28 @@ pub async fn resolve_approval(
|
||||
if let (Some(info), Some(bypass_secs)) = (info, body.bypass_secs) {
|
||||
let duration = if bypass_secs == 0 { None } else { Some(Duration::from_secs(bypass_secs)) };
|
||||
|
||||
let scope = body.bypass_scope.as_deref().unwrap_or_else(|| {
|
||||
if info.tool_category.is_some() { "category" }
|
||||
else if info.mcp_server.is_some() { "mcp_server" }
|
||||
else { "all" }
|
||||
});
|
||||
let scope = body.bypass_scope.as_deref().unwrap_or("tool");
|
||||
|
||||
// Every fallback here narrows, never widens: a scope that cannot be
|
||||
// honoured (a category-less tool, a non-MCP one) and an unknown
|
||||
// scope string both degrade to the tool itself. Only a literal
|
||||
// `"all"` disables the gate session-wide, and only because the
|
||||
// caller spelled it out.
|
||||
match scope {
|
||||
"category" => {
|
||||
if let Some(cat) = info.tool_category {
|
||||
ctx.approval.bypass_session_for_category(info.session_id, cat, duration).await;
|
||||
} else {
|
||||
apply_all_bypass(&ctx, info.session_id, duration).await;
|
||||
}
|
||||
"category" if info.tool_category.is_some() => {
|
||||
let cat = info.tool_category.unwrap();
|
||||
ctx.approval.bypass_session_for_category(info.session_id, cat, duration).await;
|
||||
}
|
||||
"mcp_server" => {
|
||||
if let Some(server) = info.mcp_server {
|
||||
ctx.approval.bypass_session_for_mcp(info.session_id, server, duration).await;
|
||||
} else {
|
||||
apply_all_bypass(&ctx, info.session_id, duration).await;
|
||||
}
|
||||
"mcp_server" if info.mcp_server.is_some() => {
|
||||
let server = info.mcp_server.clone().unwrap();
|
||||
ctx.approval.bypass_session_for_mcp(info.session_id, server, duration).await;
|
||||
}
|
||||
"all" => apply_all_bypass(&ctx, info.session_id, duration).await,
|
||||
_ => {
|
||||
ctx.approval
|
||||
.bypass_session_for_tool(info.session_id, info.tool_name.clone(), duration)
|
||||
.await;
|
||||
}
|
||||
_ => apply_all_bypass(&ctx, info.session_id, duration).await,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+2
-2
@@ -321,8 +321,8 @@ export default {
|
||||
'approval.reject': 'Deny',
|
||||
'approval.confirm_reject': 'Confirm deny',
|
||||
'approval.reject_hint': 'Optional: say why (the assistant will read it)',
|
||||
'approval.bypass_15': 'Allow and skip similar requests for 15 minutes',
|
||||
'approval.bypass_all': 'Allow and skip all requests for this session',
|
||||
'approval.bypass_15': 'Allow, and stop asking for this same tool for 15 minutes',
|
||||
'approval.bypass_all': 'Allow, and stop asking for this same tool for the rest of this conversation',
|
||||
|
||||
// ── Login ──────────────────────────────────────────────────────────────────
|
||||
'login.title': 'Welcome back',
|
||||
|
||||
+2
-2
@@ -321,8 +321,8 @@ export default {
|
||||
'approval.reject': 'Refuser',
|
||||
'approval.confirm_reject': 'Confirmer le refus',
|
||||
'approval.reject_hint': 'Facultatif : dites pourquoi (l\'assistant le lira)',
|
||||
'approval.bypass_15': 'Autoriser et ignorer les demandes similaires pendant 15 minutes',
|
||||
'approval.bypass_all': 'Autoriser et ignorer toutes les demandes pour cette session',
|
||||
'approval.bypass_15': 'Autoriser et ne plus demander pour ce même outil pendant 15 minutes',
|
||||
'approval.bypass_all': 'Autoriser et ne plus demander pour ce même outil jusqu\'à la fin de la conversation',
|
||||
|
||||
// ── Login ──────────────────────────────────────────────────────────────────
|
||||
'login.title': 'Bon retour',
|
||||
|
||||
+2
-2
@@ -321,8 +321,8 @@ export default {
|
||||
'approval.reject': 'Nega',
|
||||
'approval.confirm_reject': 'Conferma il rifiuto',
|
||||
'approval.reject_hint': 'Facoltativo: spiega perché (lo leggerà l\'assistente)',
|
||||
'approval.bypass_15': 'Consenti e salta richieste simili per 15 minuti',
|
||||
'approval.bypass_all': 'Consenti e salta tutte le richieste di questa sessione',
|
||||
'approval.bypass_15': 'Consenti e non chiedere più per questo stesso strumento per 15 minuti',
|
||||
'approval.bypass_all': 'Consenti e non chiedere più per questo stesso strumento per il resto della conversazione',
|
||||
|
||||
// ── Accesso ────────────────────────────────────────────────────────────────
|
||||
'login.title': 'Bentornato',
|
||||
|
||||
+12
-9
@@ -73,19 +73,22 @@ export const InboxCardsMixin = (Base) => class extends Base {
|
||||
this._resolveApproval(requestId, 'reject', note, null, null, toolCallId);
|
||||
}
|
||||
|
||||
/** Approve + set a timed or session bypass scoped to the tool's category or MCP server. */
|
||||
/**
|
||||
* Approve + skip approval for **this same tool** for a while.
|
||||
*
|
||||
* Scoped to the tool and nothing wider: the card the human just read is
|
||||
* about one call, and the previous category/MCP-server auto-detect meant a
|
||||
* click on "label this message" also un-gated "send this message" for the
|
||||
* rest of the session.
|
||||
*/
|
||||
_approveWithBypass(item, bypassSecs) {
|
||||
const scope = item.tool_category ? 'category'
|
||||
: item.mcp_server ? 'mcp_server'
|
||||
: 'all';
|
||||
this._resolveApproval(item.request_id, 'approve', '', bypassSecs, scope);
|
||||
this._resolveApproval(item.request_id, 'approve', '', bypassSecs, 'tool');
|
||||
}
|
||||
|
||||
/** Human-readable bypass scope label, e.g. "filesystem" or "Gmail". */
|
||||
/** Short label for the bypass scope — the tool, `mcp__x__y` shown as `y`. */
|
||||
_bypassLabel(item) {
|
||||
if (item.tool_category) return item.tool_category;
|
||||
if (item.mcp_server) return item.mcp_server;
|
||||
return 'session';
|
||||
const name = item.tool_name ?? '';
|
||||
return name.startsWith('mcp__') ? name.split('__').pop() : name;
|
||||
}
|
||||
|
||||
async _resolveClarification(requestId, inputEl) {
|
||||
|
||||
Reference in New Issue
Block a user