fix(mcp): open Kon database read-only
The kon-mcp stdio server is documented as "read-only, no auth, local-only" but until now opened the SQLite store via `kon_storage::init`, which returns a writable pool and runs migrations. Read-only-ness was enforced only by the exposed tool surface (list_transcripts, get_transcript, search_transcripts, list_tasks); a future bug or a malformed dispatch could escape into a write against the user's primary database. Add `kon_storage::init_readonly` that opens with `SqliteConnectOptions ::read_only(true)` and `create_if_missing(false)`, no migrations. The constraint is now structural — SQLite rejects writes at the connection level regardless of which handler runs. - New `init_readonly(path)` in crates/storage/src/database.rs. - Re-exported from kon_storage. - crates/mcp/src/main.rs switched over and updated startup banner. - Two tests: writes fail on the read-only pool, reads succeed; opening a non-existent DB returns an error instead of silently creating one. https://claude.ai/code/session_0189xUb6ie6t9qHkzatGZ9Rb
This commit is contained in:
@@ -7,8 +7,15 @@ use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
|
||||
#[tokio::main(flavor = "current_thread")]
|
||||
async fn main() -> anyhow::Result<()> {
|
||||
let db_path = kon_storage::database_path();
|
||||
eprintln!("[kon-mcp] opening Kon database at {}", db_path.display());
|
||||
let pool = kon_storage::init(&db_path).await?;
|
||||
eprintln!(
|
||||
"[kon-mcp] opening Kon database at {} (read-only)",
|
||||
db_path.display()
|
||||
);
|
||||
// Open read-only at the connection level so the MCP server cannot write
|
||||
// to the user's database, regardless of which tools the dispatcher
|
||||
// exposes. Migrations are deliberately skipped — this binary never owns
|
||||
// the schema; the main app is the single migration writer.
|
||||
let pool = kon_storage::init_readonly(&db_path).await?;
|
||||
eprintln!("[kon-mcp] ready, waiting for JSON-RPC on stdin");
|
||||
|
||||
let mut lines = BufReader::new(tokio::io::stdin()).lines();
|
||||
|
||||
@@ -31,6 +31,25 @@ pub async fn init(db_path: &Path) -> Result<SqlitePool> {
|
||||
Ok(pool)
|
||||
}
|
||||
|
||||
/// Open the SQLite database in read-only mode without running migrations.
|
||||
///
|
||||
/// Used by `kon-mcp` so the MCP server cannot write to the user's database
|
||||
/// regardless of which tools the dispatcher exposes — `read_only(true)` makes
|
||||
/// the constraint structural rather than relying on the request handler being
|
||||
/// well-behaved. Fails cleanly if the DB doesn't exist (no `create_if_missing`).
|
||||
pub async fn init_readonly(db_path: &Path) -> Result<SqlitePool> {
|
||||
let options = SqliteConnectOptions::new()
|
||||
.filename(db_path)
|
||||
.create_if_missing(false)
|
||||
.read_only(true);
|
||||
|
||||
SqlitePoolOptions::new()
|
||||
.max_connections(2)
|
||||
.connect_with(options)
|
||||
.await
|
||||
.map_err(|e| KonError::StorageError(format!("Read-only connect failed: {e}")))
|
||||
}
|
||||
|
||||
/// Run schema migrations via the versioned migration system.
|
||||
async fn run_migrations(pool: &SqlitePool) -> Result<()> {
|
||||
crate::migrations::run_migrations(pool).await
|
||||
@@ -2388,4 +2407,68 @@ mod tests {
|
||||
let total: u32 = series.iter().map(|d| d.count).sum();
|
||||
assert_eq!(total, 2, "exactly two completions across the window");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn init_readonly_rejects_writes_and_serves_reads() {
|
||||
let dir = std::env::temp_dir().join(format!("kon-storage-ro-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let path = dir.join("ro.db");
|
||||
let _ = std::fs::remove_file(&path);
|
||||
|
||||
// Seed via the writable path so migrations run.
|
||||
let writable = init(&path).await.unwrap();
|
||||
insert_transcript(
|
||||
&writable,
|
||||
&InsertTranscriptParams {
|
||||
id: "ro1",
|
||||
text: "seed",
|
||||
source: "microphone",
|
||||
profile_id: crate::DEFAULT_PROFILE_ID,
|
||||
title: None,
|
||||
audio_path: None,
|
||||
duration: 0.0,
|
||||
engine: None,
|
||||
model_id: None,
|
||||
inference_ms: None,
|
||||
sample_rate: None,
|
||||
audio_channels: None,
|
||||
format_mode: None,
|
||||
remove_fillers: false,
|
||||
british_english: false,
|
||||
anti_hallucination: false,
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
writable.close().await;
|
||||
|
||||
// Reopen read-only.
|
||||
let ro = init_readonly(&path).await.expect("read-only open");
|
||||
assert_eq!(count_transcripts(&ro).await.unwrap(), 1, "reads must work");
|
||||
|
||||
let write_attempt = sqlx::query("DELETE FROM transcripts WHERE id = 'ro1'")
|
||||
.execute(&ro)
|
||||
.await;
|
||||
assert!(
|
||||
write_attempt.is_err(),
|
||||
"writes must be rejected on the read-only pool",
|
||||
);
|
||||
|
||||
ro.close().await;
|
||||
let _ = std::fs::remove_file(&path);
|
||||
let _ = std::fs::remove_dir(&dir);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn init_readonly_fails_when_db_missing() {
|
||||
let path = std::env::temp_dir().join(format!(
|
||||
"kon-storage-ro-missing-{}.db",
|
||||
std::process::id()
|
||||
));
|
||||
let _ = std::fs::remove_file(&path);
|
||||
assert!(
|
||||
init_readonly(&path).await.is_err(),
|
||||
"must fail when DB file does not exist (no create_if_missing)",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,7 +10,7 @@ pub use database::{
|
||||
add_profile_term, complete_subtask_and_check_parent, complete_task, count_transcripts,
|
||||
create_profile, delete_implementation_rule, delete_profile, delete_profile_term, delete_task,
|
||||
delete_transcript, get_implementation_rule, get_profile, get_setting, get_task_by_id,
|
||||
get_transcript, init, insert_implementation_rule, insert_subtask, insert_task,
|
||||
get_transcript, init, init_readonly, insert_implementation_rule, insert_subtask, insert_task,
|
||||
insert_transcript, list_feedback_examples, list_implementation_rules, list_profile_terms,
|
||||
list_profiles, list_recent_completions, list_recent_errors, list_subtasks, list_tasks,
|
||||
list_transcripts, list_transcripts_paged, log_error, mark_implementation_rule_fired,
|
||||
|
||||
Reference in New Issue
Block a user