feat(mcp): centralize connection access policies
This commit is contained in:
56 files changed
+6393
-617
No files matched your search
@@ -1330,20 +1330,161 @@ pub fn parse_command_argv(command_text: &str) -> Result<Vec<String>, String> {
|
||||
}
|
||||
|
||||
pub fn classify_command(command: &str) -> RedisCommandSafety {
|
||||
// Read access is an explicit allowlist. Commands added by a newer Redis
|
||||
// version, module, or proxy remain high-risk until DBX reviews them.
|
||||
match command.to_ascii_uppercase().as_str() {
|
||||
"KEYS" | "FLUSHALL" | "SHUTDOWN" | "CONFIG" | "SAVE" | "BGSAVE" | "SLAVEOF" | "REPLICAOF" | "MIGRATE"
|
||||
| "MODULE" | "SCRIPT" | "EVAL" | "EVALSHA" => RedisCommandSafety::Blocked,
|
||||
"BITCOUNT"
|
||||
| "BITFIELD_RO"
|
||||
| "BITPOS"
|
||||
| "COMMAND"
|
||||
| "DBSIZE"
|
||||
| "DUMP"
|
||||
| "ECHO"
|
||||
| "EXISTS"
|
||||
| "EXPIRETIME"
|
||||
| "GEODIST"
|
||||
| "GEOHASH"
|
||||
| "GEOPOS"
|
||||
| "GEORADIUS_RO"
|
||||
| "GEORADIUSBYMEMBER_RO"
|
||||
| "GEOSEARCH"
|
||||
| "GET"
|
||||
| "GETBIT"
|
||||
| "GETRANGE"
|
||||
| "HEXISTS"
|
||||
| "HGET"
|
||||
| "HGETALL"
|
||||
| "HKEYS"
|
||||
| "HLEN"
|
||||
| "HMGET"
|
||||
| "HRANDFIELD"
|
||||
| "HSCAN"
|
||||
| "HSTRLEN"
|
||||
| "HVALS"
|
||||
| "INFO"
|
||||
| "LASTSAVE"
|
||||
| "LCS"
|
||||
| "LINDEX"
|
||||
| "LLEN"
|
||||
| "LPOS"
|
||||
| "LRANGE"
|
||||
| "MGET"
|
||||
| "OBJECT"
|
||||
| "PEXPIRETIME"
|
||||
| "PFCOUNT"
|
||||
| "PING"
|
||||
| "PTTL"
|
||||
| "PUBSUB"
|
||||
| "RANDOMKEY"
|
||||
| "ROLE"
|
||||
| "SCAN"
|
||||
| "SCARD"
|
||||
| "SDIFF"
|
||||
| "SINTER"
|
||||
| "SINTERCARD"
|
||||
| "SISMEMBER"
|
||||
| "SMEMBERS"
|
||||
| "SMISMEMBER"
|
||||
| "SORT_RO"
|
||||
| "SRANDMEMBER"
|
||||
| "SSCAN"
|
||||
| "STRLEN"
|
||||
| "SUNION"
|
||||
| "TIME"
|
||||
| "TTL"
|
||||
| "TYPE"
|
||||
| "WAIT"
|
||||
| "WAITAOF"
|
||||
| "XINFO"
|
||||
| "XLEN"
|
||||
| "XPENDING"
|
||||
| "XRANGE"
|
||||
| "XREAD"
|
||||
| "XREVRANGE"
|
||||
| "ZCARD"
|
||||
| "ZCOUNT"
|
||||
| "ZDIFF"
|
||||
| "ZINTER"
|
||||
| "ZINTERCARD"
|
||||
| "ZLEXCOUNT"
|
||||
| "ZMSCORE"
|
||||
| "ZRANDMEMBER"
|
||||
| "ZRANGE"
|
||||
| "ZRANGEBYLEX"
|
||||
| "ZRANGEBYSCORE"
|
||||
| "ZRANK"
|
||||
| "ZREVRANGE"
|
||||
| "ZREVRANGEBYLEX"
|
||||
| "ZREVRANGEBYSCORE"
|
||||
| "ZREVRANK"
|
||||
| "ZSCAN"
|
||||
| "ZSCORE"
|
||||
| "ZUNION"
|
||||
| "BF.CARD"
|
||||
| "BF.EXISTS"
|
||||
| "BF.INFO"
|
||||
| "BF.MEXISTS"
|
||||
| "CF.COUNT"
|
||||
| "CF.EXISTS"
|
||||
| "CF.INFO"
|
||||
| "CF.MEXISTS"
|
||||
| "CMS.INFO"
|
||||
| "CMS.QUERY"
|
||||
| "FT._LIST"
|
||||
| "FT.AGGREGATE"
|
||||
| "FT.DICTDUMP"
|
||||
| "FT.EXPLAIN"
|
||||
| "FT.EXPLAINCLI"
|
||||
| "FT.INFO"
|
||||
| "FT.PROFILE"
|
||||
| "FT.SEARCH"
|
||||
| "FT.SPELLCHECK"
|
||||
| "FT.SYNDUMP"
|
||||
| "FT.TAGVALS"
|
||||
| "GRAPH.RO_QUERY"
|
||||
| "JSON.ARRINDEX"
|
||||
| "JSON.ARRLEN"
|
||||
| "JSON.GET"
|
||||
| "JSON.MGET"
|
||||
| "JSON.OBJKEYS"
|
||||
| "JSON.OBJLEN"
|
||||
| "JSON.RESP"
|
||||
| "JSON.STRLEN"
|
||||
| "JSON.TYPE"
|
||||
| "TDIGEST.BYRANK"
|
||||
| "TDIGEST.BYREVRANK"
|
||||
| "TDIGEST.CDF"
|
||||
| "TDIGEST.INFO"
|
||||
| "TDIGEST.MAX"
|
||||
| "TDIGEST.MIN"
|
||||
| "TDIGEST.QUANTILE"
|
||||
| "TDIGEST.RANK"
|
||||
| "TDIGEST.REVRANK"
|
||||
| "TDIGEST.TRIMMED_MEAN"
|
||||
| "TOPK.INFO"
|
||||
| "TOPK.LIST"
|
||||
| "TOPK.QUERY"
|
||||
| "TS.GET"
|
||||
| "TS.INFO"
|
||||
| "TS.MGET"
|
||||
| "TS.MRANGE"
|
||||
| "TS.QUERYINDEX"
|
||||
| "TS.RANGE" => RedisCommandSafety::Allowed,
|
||||
"DEL" | "UNLINK" | "EXPIRE" | "EXPIREAT" | "PEXPIRE" | "PEXPIREAT" | "RENAME" | "RENAMENX" | "GETDEL"
|
||||
| "HDEL" | "LPOP" | "RPOP" | "LREM" | "LTRIM" | "SPOP" | "SREM" | "ZREM" | "ZPOPMAX" | "ZPOPMIN" | "ZMPOP"
|
||||
| "HDEL" | "JSON.ARRPOP" | "JSON.ARRTRIM" | "JSON.CLEAR" | "JSON.DEL" | "JSON.FORGET" | "BLMOVE" | "BLMPOP"
|
||||
| "BLPOP" | "BRPOP" | "BRPOPLPUSH" | "LPOP" | "LMOVE" | "LMPOP" | "RPOP" | "RPOPLPUSH" | "LREM" | "LTRIM"
|
||||
| "SPOP" | "SREM" | "ZREM" | "ZPOPMAX" | "ZPOPMIN" | "ZMPOP" | "BZMPOP" | "BZPOPMAX" | "BZPOPMIN"
|
||||
| "ZREMRANGEBYLEX" | "ZREMRANGEBYRANK" | "ZREMRANGEBYSCORE" | "XDEL" | "XTRIM" | "MOVE" | "SORT"
|
||||
| "SDIFFSTORE" | "SINTERSTORE" | "SUNIONSTORE" | "ZDIFFSTORE" | "ZINTERSTORE" | "ZRANGESTORE"
|
||||
| "ZUNIONSTORE" | "PFMERGE" | "GEOSEARCHSTORE" | "FLUSHDB" => RedisCommandSafety::Confirm,
|
||||
| "ZUNIONSTORE" | "PFMERGE" | "GEOSEARCHSTORE" => RedisCommandSafety::Confirm,
|
||||
"APPEND" | "BITFIELD" | "BITOP" | "COPY" | "DECR" | "DECRBY" | "GEOADD" | "GEORADIUS" | "GEORADIUSBYMEMBER"
|
||||
| "GETSET" | "INCR" | "INCRBY" | "INCRBYFLOAT" | "SET" | "SETEX" | "PSETEX" | "SETNX" | "SETRANGE" | "MSET"
|
||||
| "MSETNX" | "PERSIST" | "HSET" | "HMSET" | "HINCRBY" | "HINCRBYFLOAT" | "HSETNX" | "LINSERT" | "LSET"
|
||||
| "LMOVE" | "LPUSH" | "LPUSHX" | "PFADD" | "RPUSH" | "RPUSHX" | "RESTORE" | "SADD" | "ZADD" | "ZINCRBY"
|
||||
| "SETBIT" | "XADD" | "XACK" | "XAUTOCLAIM" | "XCLAIM" | "XSETID" => RedisCommandSafety::Write,
|
||||
_ => RedisCommandSafety::Allowed,
|
||||
| "GETEX" | "GETSET" | "INCR" | "INCRBY" | "INCRBYFLOAT" | "SET" | "SETEX" | "PSETEX" | "SETNX"
|
||||
| "SETRANGE" | "MSET" | "MSETNX" | "PERSIST" | "HSET" | "HMSET" | "HINCRBY" | "HINCRBYFLOAT" | "HSETNX"
|
||||
| "JSON.ARRAPPEND" | "JSON.ARRINSERT" | "JSON.MERGE" | "JSON.MSET" | "JSON.NUMINCRBY" | "JSON.NUMMULTBY"
|
||||
| "JSON.SET" | "JSON.STRAPPEND" | "JSON.TOGGLE" | "LINSERT" | "LSET" | "LPUSH" | "LPUSHX" | "PFADD"
|
||||
| "RPUSH" | "RPUSHX" | "RESTORE" | "SADD" | "ZADD" | "ZINCRBY" | "SETBIT" | "SPUBLISH" | "PUBLISH"
|
||||
| "TOUCH" | "XADD" | "XACK" | "XAUTOCLAIM" | "XCLAIM" | "XREADGROUP" | "XSETID" => RedisCommandSafety::Write,
|
||||
_ => RedisCommandSafety::Blocked,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2971,11 +3112,31 @@ mod tests {
|
||||
assert_eq!(argv, vec!["SET", "user:1", "Ada \"Lovelace\""]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_quoted_and_escaped_command_names_before_classification() {
|
||||
for command_text in [r#""JSON.SET" user:1 $ {}"#, r#"JSON\.SET user:1 $ {}"#] {
|
||||
let argv = parse_command_argv(command_text).unwrap();
|
||||
assert_eq!(classify_command(&argv[0]), RedisCommandSafety::Write);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_empty_command_text() {
|
||||
assert_eq!(parse_command_argv(" ").unwrap_err(), "Redis command is empty");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn raw_command_execution_fails_closed_before_sending_invalid_or_unknown_commands() {
|
||||
let mut con = FakeRedisConnection::new(Vec::new());
|
||||
|
||||
let unknown = super::execute_command(&mut con, "VENDOR.WRITE key value", false).await.unwrap_err();
|
||||
assert!(unknown.contains("blocked for safety"));
|
||||
|
||||
let malformed = super::execute_command(&mut con, r#"GET "unterminated"#, true).await.unwrap_err();
|
||||
assert_eq!(malformed, "Redis command has an unterminated quote");
|
||||
assert!(con.commands.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn matches_redis_values_case_insensitively() {
|
||||
assert!(redis_value_matches_query(&string_value("Hello Redis"), "redis"));
|
||||
@@ -3040,13 +3201,20 @@ mod tests {
|
||||
#[test]
|
||||
fn classifies_safe_confirmed_and_blocked_commands() {
|
||||
assert_eq!(classify_command("GET"), RedisCommandSafety::Allowed);
|
||||
assert_eq!(classify_command("JSON.GET"), RedisCommandSafety::Allowed);
|
||||
assert_eq!(classify_command("set"), RedisCommandSafety::Write);
|
||||
assert_eq!(classify_command("hset"), RedisCommandSafety::Write);
|
||||
assert_eq!(classify_command("JSON.SET"), RedisCommandSafety::Write);
|
||||
assert_eq!(classify_command("GETEX"), RedisCommandSafety::Write);
|
||||
assert_eq!(classify_command("XREADGROUP"), RedisCommandSafety::Write);
|
||||
assert_eq!(classify_command("del"), RedisCommandSafety::Confirm);
|
||||
assert_eq!(classify_command("flushdb"), RedisCommandSafety::Confirm);
|
||||
assert_eq!(classify_command("flushdb"), RedisCommandSafety::Blocked);
|
||||
assert_eq!(classify_command("KEYS"), RedisCommandSafety::Blocked);
|
||||
assert_eq!(classify_command("flushall"), RedisCommandSafety::Blocked);
|
||||
assert_eq!(classify_command("eval"), RedisCommandSafety::Blocked);
|
||||
assert_eq!(classify_command("FCALL"), RedisCommandSafety::Blocked);
|
||||
assert_eq!(classify_command("XGROUP"), RedisCommandSafety::Blocked);
|
||||
assert_eq!(classify_command("VENDOR.WRITE"), RedisCommandSafety::Blocked);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -2085,6 +2085,24 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_mcp_access_is_ignored_and_not_serialized() {
|
||||
let legacy: ConnectionConfig = serde_json::from_value(serde_json::json!({
|
||||
"id": "legacy",
|
||||
"name": "Legacy",
|
||||
"db_type": "mysql",
|
||||
"host": "127.0.0.1",
|
||||
"port": 3306,
|
||||
"username": "root",
|
||||
"password": "",
|
||||
"database": null,
|
||||
"mcp_access": "read_only"
|
||||
}))
|
||||
.unwrap();
|
||||
assert!(serde_json::to_value(&legacy).unwrap().get("mcp_access").is_none());
|
||||
assert!(!legacy.read_only);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn database_identifier_whitespace_is_preserved_and_percent_encoded() {
|
||||
let mut config = mysql_config("root", "secret", Some(" analytics "));
|
||||
|
||||
@@ -86,6 +86,17 @@ impl MongoCommand {
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn has_effectively_unbounded_filter(&self) -> bool {
|
||||
match self {
|
||||
Self::Update { filter, .. }
|
||||
| Self::Delete { filter, .. }
|
||||
| Self::FindOneAndUpdate { filter, .. }
|
||||
| Self::FindOneAndReplace { filter, .. }
|
||||
| Self::FindOneAndDelete { filter, .. } => mongo_filter_is_effectively_unbounded(filter),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn validate_safety(
|
||||
@@ -97,7 +108,7 @@ pub fn validate_safety(
|
||||
if command.is_mutating() && !allow_writes {
|
||||
return Err(MongoSafetyError::WritesDisabled);
|
||||
}
|
||||
if command.has_empty_filter() && !allow_dangerous {
|
||||
if command.has_effectively_unbounded_filter() && !allow_dangerous {
|
||||
return Err(MongoSafetyError::EmptyFilter);
|
||||
}
|
||||
if command.is_dangerous() && !allow_dangerous {
|
||||
@@ -109,6 +120,207 @@ pub fn validate_safety(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn mongo_filter_is_effectively_unbounded(filter_json: &str) -> bool {
|
||||
serde_json::from_str::<serde_json::Value>(filter_json)
|
||||
.ok()
|
||||
.as_ref()
|
||||
.is_none_or(|value| mongo_filter_contains_opaque_logic(value) || mongo_filter_value_is_unbounded(value))
|
||||
}
|
||||
|
||||
fn mongo_filter_contains_opaque_logic(value: &serde_json::Value) -> bool {
|
||||
let Some(filter) = value.as_object() else {
|
||||
return true;
|
||||
};
|
||||
filter.iter().any(|(key, value)| match key.as_str() {
|
||||
"$comment" => false,
|
||||
"$where" | "$expr" | "$nor" => true,
|
||||
"$and" | "$or" => {
|
||||
let Some(clauses) = value.as_array() else {
|
||||
return true;
|
||||
};
|
||||
clauses.is_empty()
|
||||
|| clauses.iter().any(|clause| !clause.is_object() || mongo_filter_contains_opaque_logic(clause))
|
||||
|| (key == "$or"
|
||||
&& clauses
|
||||
.iter()
|
||||
.any(|clause| clause.as_object().is_some_and(|document| document.contains_key("$and"))))
|
||||
|| (key == "$or" && mongo_or_has_complementary_field_clauses(clauses))
|
||||
}
|
||||
_ => key.starts_with('$') || mongo_field_predicate_contains_opaque_logic(value),
|
||||
})
|
||||
}
|
||||
|
||||
fn mongo_field_predicate_contains_opaque_logic(value: &serde_json::Value) -> bool {
|
||||
let Some(predicate) = value.as_object() else {
|
||||
return false;
|
||||
};
|
||||
if mongo_extended_json_scalar_literal_is_valid(value) {
|
||||
return false;
|
||||
}
|
||||
let has_operator = predicate.keys().any(|key| key.starts_with('$'));
|
||||
has_operator
|
||||
&& predicate.keys().any(|key| {
|
||||
!matches!(key.as_str(), "$eq" | "$ne" | "$gt" | "$gte" | "$lt" | "$lte" | "$in" | "$nin" | "$exists")
|
||||
})
|
||||
}
|
||||
|
||||
fn mongo_extended_json_scalar_literal_is_valid(value: &serde_json::Value) -> bool {
|
||||
let Some(wrapper) = value.as_object().filter(|wrapper| wrapper.len() == 1) else {
|
||||
return false;
|
||||
};
|
||||
if let Some(value) = wrapper.get("$oid").and_then(serde_json::Value::as_str) {
|
||||
return value.len() == 24 && value.bytes().all(|byte| byte.is_ascii_hexdigit());
|
||||
}
|
||||
if let Some(value) = wrapper.get("$numberLong").and_then(serde_json::Value::as_str) {
|
||||
return value.parse::<i64>().is_ok();
|
||||
}
|
||||
wrapper
|
||||
.get("$date")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.is_some_and(|value| chrono::DateTime::parse_from_rfc3339(value).is_ok())
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, PartialEq, Eq)]
|
||||
enum MongoFieldOperator {
|
||||
Eq,
|
||||
Ne,
|
||||
Gt,
|
||||
Gte,
|
||||
Lt,
|
||||
Lte,
|
||||
In,
|
||||
Nin,
|
||||
Exists,
|
||||
}
|
||||
|
||||
struct MongoPureFieldPredicate<'a> {
|
||||
field: &'a str,
|
||||
operator: MongoFieldOperator,
|
||||
operand: &'a serde_json::Value,
|
||||
}
|
||||
|
||||
fn mongo_or_has_complementary_field_clauses(clauses: &[serde_json::Value]) -> bool {
|
||||
clauses.iter().enumerate().any(|(index, clause)| {
|
||||
let Some(predicate) = mongo_pure_field_predicate(clause) else {
|
||||
return false;
|
||||
};
|
||||
clauses[index + 1..]
|
||||
.iter()
|
||||
.filter_map(mongo_pure_field_predicate)
|
||||
.any(|other| mongo_field_predicates_are_complementary(&predicate, &other))
|
||||
})
|
||||
}
|
||||
|
||||
fn mongo_pure_field_predicate(value: &serde_json::Value) -> Option<MongoPureFieldPredicate<'_>> {
|
||||
let filter = value.as_object()?;
|
||||
let mut entries = filter.iter().filter(|(key, _)| key.as_str() != "$comment");
|
||||
let (field, predicate) = entries.next()?;
|
||||
if entries.next().is_some() {
|
||||
return None;
|
||||
}
|
||||
if field == "$and" {
|
||||
let clauses = predicate.as_array()?;
|
||||
let mut bounded = clauses.iter().filter(|clause| !mongo_filter_value_is_unbounded(clause));
|
||||
let clause = bounded.next()?;
|
||||
if bounded.next().is_some() {
|
||||
return None;
|
||||
}
|
||||
return mongo_pure_field_predicate(clause);
|
||||
}
|
||||
if field == "$or" {
|
||||
let clauses = predicate.as_array()?;
|
||||
return (clauses.len() == 1).then(|| mongo_pure_field_predicate(&clauses[0])).flatten();
|
||||
}
|
||||
if field.starts_with('$') {
|
||||
return None;
|
||||
}
|
||||
let Some(operator_document) = predicate.as_object() else {
|
||||
return Some(MongoPureFieldPredicate { field, operator: MongoFieldOperator::Eq, operand: predicate });
|
||||
};
|
||||
if mongo_extended_json_scalar_literal_is_valid(predicate)
|
||||
|| !operator_document.keys().any(|key| key.starts_with('$'))
|
||||
{
|
||||
return Some(MongoPureFieldPredicate { field, operator: MongoFieldOperator::Eq, operand: predicate });
|
||||
}
|
||||
let mut operators = operator_document.iter();
|
||||
let (operator, operand) = operators.next()?;
|
||||
if operators.next().is_some() {
|
||||
return None;
|
||||
}
|
||||
let operator = match operator.as_str() {
|
||||
"$eq" => MongoFieldOperator::Eq,
|
||||
"$ne" => MongoFieldOperator::Ne,
|
||||
"$gt" => MongoFieldOperator::Gt,
|
||||
"$gte" => MongoFieldOperator::Gte,
|
||||
"$lt" => MongoFieldOperator::Lt,
|
||||
"$lte" => MongoFieldOperator::Lte,
|
||||
"$in" => MongoFieldOperator::In,
|
||||
"$nin" => MongoFieldOperator::Nin,
|
||||
"$exists" => MongoFieldOperator::Exists,
|
||||
_ => return None,
|
||||
};
|
||||
Some(MongoPureFieldPredicate { field, operator, operand })
|
||||
}
|
||||
|
||||
fn mongo_field_predicates_are_complementary(
|
||||
left: &MongoPureFieldPredicate<'_>,
|
||||
right: &MongoPureFieldPredicate<'_>,
|
||||
) -> bool {
|
||||
if left.field != right.field {
|
||||
return false;
|
||||
}
|
||||
use MongoFieldOperator::{Eq, Exists, Gt, Gte, In, Lt, Lte, Ne, Nin};
|
||||
match (left.operator, right.operator) {
|
||||
(Exists, Exists) => {
|
||||
left.operand.as_bool().zip(right.operand.as_bool()).is_some_and(|(left, right)| left != right)
|
||||
}
|
||||
(In, Nin) | (Nin, In) => mongo_json_sets_equal(left.operand, right.operand),
|
||||
(Eq, Ne) | (Ne, Eq) | (Gt, Lte) | (Lte, Gt) | (Gte, Lt) | (Lt, Gte) => left.operand == right.operand,
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn mongo_json_sets_equal(left: &serde_json::Value, right: &serde_json::Value) -> bool {
|
||||
let (Some(left), Some(right)) = (left.as_array(), right.as_array()) else {
|
||||
return false;
|
||||
};
|
||||
left.iter().all(|value| right.contains(value)) && right.iter().all(|value| left.contains(value))
|
||||
}
|
||||
|
||||
fn mongo_filter_value_is_unbounded(value: &serde_json::Value) -> bool {
|
||||
let Some(filter) = value.as_object() else {
|
||||
return true;
|
||||
};
|
||||
if filter.is_empty() || filter.contains_key("$where") || filter.contains_key("$expr") {
|
||||
return true;
|
||||
}
|
||||
filter.iter().all(|(key, value)| match key.as_str() {
|
||||
"$comment" => true,
|
||||
"$and" => value
|
||||
.as_array()
|
||||
.is_none_or(|clauses| clauses.is_empty() || clauses.iter().all(mongo_filter_value_is_unbounded)),
|
||||
"$or" => value
|
||||
.as_array()
|
||||
.is_none_or(|clauses| clauses.is_empty() || clauses.iter().any(mongo_filter_value_is_unbounded)),
|
||||
"$nor" => true,
|
||||
_ if mongo_field_predicate_is_empty_nin(value) => true,
|
||||
"_id" if mongo_field_predicate_is_exists_true(value) => true,
|
||||
_ => key.starts_with('$'),
|
||||
})
|
||||
}
|
||||
|
||||
fn mongo_field_predicate_is_empty_nin(value: &serde_json::Value) -> bool {
|
||||
value.as_object().is_some_and(|predicate| {
|
||||
predicate.len() == 1 && predicate.get("$nin").and_then(serde_json::Value::as_array).is_some_and(Vec::is_empty)
|
||||
})
|
||||
}
|
||||
|
||||
fn mongo_field_predicate_is_exists_true(value: &serde_json::Value) -> bool {
|
||||
value.as_object().is_some_and(|predicate| {
|
||||
predicate.len() == 1 && predicate.get("$exists").and_then(serde_json::Value::as_bool) == Some(true)
|
||||
})
|
||||
}
|
||||
|
||||
pub fn parse(input: &str) -> Result<MongoCommand, String> {
|
||||
let source = input.trim().trim_end_matches(';').trim();
|
||||
if source.eq_ignore_ascii_case("db.version()") {
|
||||
@@ -598,6 +810,33 @@ mod tests {
|
||||
assert!(update.has_empty_filter());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn treats_effectively_unbounded_write_filters_as_dangerous() {
|
||||
for command in [
|
||||
r#"db.items.deleteMany({_id: {$exists: true}})"#,
|
||||
r#"db.items.deleteMany({id: {$nin: []}})"#,
|
||||
r#"db.items.deleteMany({$expr: true})"#,
|
||||
r#"db.items.deleteMany({$or: [{id: 1}, {id: {$ne: 1}}]})"#,
|
||||
] {
|
||||
let command = parse(command).unwrap();
|
||||
assert!(command.has_effectively_unbounded_filter(), "{command:?}");
|
||||
assert_eq!(
|
||||
validate_safety(&command, true, false, false),
|
||||
Err(MongoSafetyError::EmptyFilter),
|
||||
"{command:?}"
|
||||
);
|
||||
}
|
||||
|
||||
for command in [
|
||||
r#"db.items.deleteMany({_id: ObjectId('507f1f77bcf86cd799439011')})"#,
|
||||
r#"db.items.updateMany({tenant_id: 7}, {$set: {active: false}})"#,
|
||||
] {
|
||||
let command = parse(command).unwrap();
|
||||
assert!(!command.has_effectively_unbounded_filter(), "{command:?}");
|
||||
assert_eq!(validate_safety(&command, true, false, false), Ok(()), "{command:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn accepts_multiline_chains_and_update_options() {
|
||||
let command = parse(
|
||||
|
||||
@@ -104,6 +104,78 @@ pub fn is_production_database(config: &ConnectionConfig, database: &str) -> bool
|
||||
.any(|name| normalize_database_name(name) == normalize_database_name(database)))
|
||||
}
|
||||
|
||||
/// Returns whether a MongoDB aggregation write stage targets production scope.
|
||||
/// `$out` and `$merge` can name a database different from the pipeline's source
|
||||
/// database, so checking only the selected database is insufficient.
|
||||
pub fn mongo_pipeline_targets_production_database(
|
||||
config: &ConnectionConfig,
|
||||
active_database: &str,
|
||||
pipeline_json: &str,
|
||||
) -> bool {
|
||||
if is_production_database(config, active_database) {
|
||||
return true;
|
||||
}
|
||||
|
||||
let Ok(serde_json::Value::Array(stages)) = serde_json::from_str::<serde_json::Value>(pipeline_json) else {
|
||||
return true;
|
||||
};
|
||||
let mut has_write_stage = false;
|
||||
let mut uncertain = false;
|
||||
let mut targets = HashSet::new();
|
||||
|
||||
for stage in stages {
|
||||
let Some(document) = stage.as_object() else {
|
||||
continue;
|
||||
};
|
||||
if let Some(target) = document.get("$out") {
|
||||
has_write_stage = true;
|
||||
match target {
|
||||
serde_json::Value::String(_) => {
|
||||
add_mongo_current_database(&mut targets, &mut uncertain, active_database)
|
||||
}
|
||||
serde_json::Value::Object(target) => match target.get("db").and_then(serde_json::Value::as_str) {
|
||||
Some(database) if !database.trim().is_empty() => {
|
||||
targets.insert(database.to_string());
|
||||
}
|
||||
_ => uncertain = true,
|
||||
},
|
||||
_ => uncertain = true,
|
||||
}
|
||||
}
|
||||
if let Some(target) = document.get("$merge") {
|
||||
has_write_stage = true;
|
||||
match target {
|
||||
serde_json::Value::String(_) => {
|
||||
add_mongo_current_database(&mut targets, &mut uncertain, active_database)
|
||||
}
|
||||
serde_json::Value::Object(target) => match target.get("into") {
|
||||
Some(serde_json::Value::String(_)) => {
|
||||
add_mongo_current_database(&mut targets, &mut uncertain, active_database)
|
||||
}
|
||||
Some(serde_json::Value::Object(into)) => match into.get("db").and_then(serde_json::Value::as_str) {
|
||||
Some(database) if !database.trim().is_empty() => {
|
||||
targets.insert(database.to_string());
|
||||
}
|
||||
_ => uncertain = true,
|
||||
},
|
||||
_ => uncertain = true,
|
||||
},
|
||||
_ => uncertain = true,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
has_write_stage && (uncertain || targets.iter().any(|database| is_production_database(config, database)))
|
||||
}
|
||||
|
||||
fn add_mongo_current_database(targets: &mut HashSet<String>, uncertain: &mut bool, active_database: &str) {
|
||||
if active_database.trim().is_empty() {
|
||||
*uncertain = true;
|
||||
} else {
|
||||
targets.insert(active_database.to_string());
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns whether a non-read SQL statement targets production scope.
|
||||
///
|
||||
/// Agent execution already classifies SQL risk with `sql_risk`; this function
|
||||
@@ -144,6 +216,7 @@ fn referenced_databases(sql: &str, db_type: &DatabaseType, active_database: &str
|
||||
.filter(|value| !value.is_empty())
|
||||
{
|
||||
use_database = database;
|
||||
assessment.databases.insert(use_database.clone());
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -547,7 +620,7 @@ fn append_quoted_identifier_token(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{is_production_database, targets_production_database};
|
||||
use super::{is_production_database, mongo_pipeline_targets_production_database, targets_production_database};
|
||||
use crate::models::connection::{ConnectionConfig, DatabaseType};
|
||||
use serde::Deserialize;
|
||||
|
||||
@@ -649,6 +722,7 @@ mod tests {
|
||||
"staging",
|
||||
"SELECT * FROM prod_app.users; DELETE FROM staging.users WHERE id = 1"
|
||||
));
|
||||
assert!(targets_production_database(&config(), "staging", "USE prod_app"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -686,4 +760,40 @@ mod tests {
|
||||
assert!(targets_production_database(&sqlserver, "staging", "DELETE FROM prod_app.dbo.users WHERE id = 1"));
|
||||
assert!(!targets_production_database(&sqlserver, "staging", "DELETE FROM prod_app.users WHERE id = 1"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn detects_cross_database_mongo_aggregate_write_targets() {
|
||||
let mut mongo = config();
|
||||
mongo.db_type = DatabaseType::MongoDb;
|
||||
mongo.database = Some("staging".to_string());
|
||||
mongo.production_databases = vec!["production".to_string()];
|
||||
|
||||
assert!(mongo_pipeline_targets_production_database(
|
||||
&mongo,
|
||||
"staging",
|
||||
r#"[{"$out":{"db":"production","coll":"copied"}}]"#
|
||||
));
|
||||
assert!(mongo_pipeline_targets_production_database(
|
||||
&mongo,
|
||||
"staging",
|
||||
r#"[{"$merge":{"into":{"db":"production","coll":"copied"}}}]"#
|
||||
));
|
||||
assert!(!mongo_pipeline_targets_production_database(&mongo, "staging", r#"[{"$out":"copied"}]"#));
|
||||
assert!(mongo_pipeline_targets_production_database(&mongo, "production", r#"[{"$merge":{"into":"copied"}}]"#));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fails_closed_for_indeterminate_mongo_aggregate_write_targets() {
|
||||
let mut mongo = config();
|
||||
mongo.db_type = DatabaseType::MongoDb;
|
||||
mongo.database = None;
|
||||
mongo.production_databases = vec!["production".to_string()];
|
||||
|
||||
assert!(mongo_pipeline_targets_production_database(&mongo, "", r#"[{"$out":"copied"}]"#));
|
||||
assert!(mongo_pipeline_targets_production_database(
|
||||
&mongo,
|
||||
"staging",
|
||||
r#"[{"$merge":{"whenMatched":"replace"}}]"#
|
||||
));
|
||||
}
|
||||
}
|
||||
@@ -474,7 +474,7 @@ fn has_extra_statement_after_semicolon(sql: &str) -> bool {
|
||||
stripped.split(';').skip(1).any(|part| !part.trim().is_empty())
|
||||
}
|
||||
|
||||
fn strip_sql_comments(sql: &str) -> String {
|
||||
pub(crate) fn strip_sql_comments(sql: &str) -> String {
|
||||
let mut output = String::with_capacity(sql.len());
|
||||
let mut chars = sql.chars().peekable();
|
||||
let mut in_line_comment = false;
|
||||
|
||||
+913
-11
@@ -1,9 +1,15 @@
|
||||
use std::ops::ControlFlow;
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
use sqlparser::ast::{Query, SetExpr, Statement};
|
||||
use sqlparser::ast::{
|
||||
visit_expressions, BinaryOperator, Expr, FromTable, OnConflictAction, OnInsert, Query, SetExpr, SqliteOnConflict,
|
||||
Statement, UnaryOperator, Value,
|
||||
};
|
||||
use sqlparser::dialect::{
|
||||
ClickHouseDialect, DuckDbDialect, GenericDialect, MsSqlDialect, MySqlDialect, PostgreSqlDialect, SQLiteDialect,
|
||||
};
|
||||
use sqlparser::parser::Parser;
|
||||
use sqlparser::tokenizer::{Token, Tokenizer};
|
||||
|
||||
use crate::models::connection::DatabaseType;
|
||||
|
||||
@@ -64,7 +70,7 @@ fn classify_statement(stmt: &Statement, detect_select_into: bool) -> SqlRisk {
|
||||
match stmt {
|
||||
// Pure reads
|
||||
Statement::Query(query) => {
|
||||
if detect_select_into && query_contains_select_into(query) {
|
||||
if query_is_write_capable(query, detect_select_into) {
|
||||
SqlRisk::Write
|
||||
} else {
|
||||
SqlRisk::ReadOnly
|
||||
@@ -128,21 +134,531 @@ fn classify_statement(stmt: &Statement, detect_select_into: bool) -> SqlRisk {
|
||||
}
|
||||
}
|
||||
|
||||
fn query_contains_select_into(query: &Query) -> bool {
|
||||
set_expr_contains_select_into(&query.body)
|
||||
fn statement_is_dangerous(stmt: &Statement, detect_select_into: bool) -> bool {
|
||||
match stmt {
|
||||
Statement::Query(query) => query_is_dangerous(query, detect_select_into),
|
||||
Statement::Insert(insert) => {
|
||||
insert.replace_into
|
||||
|| insert.overwrite
|
||||
|| matches!(insert.or, Some(SqliteOnConflict::Replace))
|
||||
|| insert.on.as_ref().is_some_and(|on| match on {
|
||||
OnInsert::DuplicateKeyUpdate(_) => true,
|
||||
OnInsert::OnConflict(conflict) => matches!(conflict.action, OnConflictAction::DoUpdate(_)),
|
||||
_ => true,
|
||||
})
|
||||
|| insert.source.as_ref().is_some_and(|query| !matches!(query.body.as_ref(), SetExpr::Values(_)))
|
||||
}
|
||||
Statement::Update(update) => {
|
||||
!update.table.joins.is_empty()
|
||||
|| update.from.is_some()
|
||||
|| matches!(update.or, Some(SqliteOnConflict::Replace))
|
||||
|| update.selection.as_ref().is_none_or(predicate_is_obviously_unbounded)
|
||||
}
|
||||
Statement::Delete(delete) => {
|
||||
let from = match &delete.from {
|
||||
FromTable::WithFromKeyword(from) | FromTable::WithoutKeyword(from) => from,
|
||||
};
|
||||
!delete.tables.is_empty()
|
||||
|| delete.using.is_some()
|
||||
|| from.len() != 1
|
||||
|| from.first().is_none_or(|table| !table.joins.is_empty())
|
||||
|| delete.selection.as_ref().is_none_or(predicate_is_obviously_unbounded)
|
||||
}
|
||||
Statement::Merge { .. } => true,
|
||||
Statement::Explain { analyze: true, statement, .. } => statement_is_dangerous(statement, detect_select_into),
|
||||
_ => !matches!(classify_statement(stmt, detect_select_into), SqlRisk::ReadOnly),
|
||||
}
|
||||
}
|
||||
|
||||
fn set_expr_contains_select_into(expr: &SetExpr) -> bool {
|
||||
fn predicate_is_obviously_unbounded(expr: &Expr) -> bool {
|
||||
if expr_contains_subquery(expr) {
|
||||
return true;
|
||||
}
|
||||
if !expr_contains_column_reference(expr) {
|
||||
return true;
|
||||
}
|
||||
|
||||
match expr {
|
||||
SetExpr::Select(select) => select.into.is_some(),
|
||||
SetExpr::Query(query) => query_contains_select_into(query),
|
||||
SetExpr::SetOperation { left, right, .. } => {
|
||||
set_expr_contains_select_into(left) || set_expr_contains_select_into(right)
|
||||
Expr::Nested(expr) => predicate_is_obviously_unbounded(expr),
|
||||
Expr::Value(value) => value_is_truthy(&value.value),
|
||||
Expr::UnaryOp { op: UnaryOperator::Not, expr } => predicate_is_obviously_false(expr),
|
||||
Expr::BinaryOp { left, op: BinaryOperator::And, right } => {
|
||||
predicate_is_obviously_unbounded(left) && predicate_is_obviously_unbounded(right)
|
||||
}
|
||||
Expr::BinaryOp { left, op: BinaryOperator::Or, right } => {
|
||||
or_contains_conjunctive_branch(expr)
|
||||
|| or_contains_complementary_null_checks(expr)
|
||||
|| or_contains_complementary_comparisons(expr)
|
||||
|| or_contains_complementary_in_lists(expr)
|
||||
|| or_contains_complementary_between_checks(expr)
|
||||
|| predicate_is_obviously_unbounded(left)
|
||||
|| predicate_is_obviously_unbounded(right)
|
||||
}
|
||||
Expr::BinaryOp { left, op, right } if is_comparison_operator(op) => constant_comparison_truth(left, op, right)
|
||||
.unwrap_or_else(|| {
|
||||
left == right
|
||||
&& matches!(
|
||||
op,
|
||||
BinaryOperator::Eq | BinaryOperator::GtEq | BinaryOperator::LtEq | BinaryOperator::Spaceship
|
||||
)
|
||||
}),
|
||||
Expr::IsNotDistinctFrom(left, right) => strip_nested_expr(left) == strip_nested_expr(right),
|
||||
Expr::IsTrue(expr) | Expr::IsNotFalse(expr) => predicate_is_obviously_unbounded(expr),
|
||||
Expr::Like { negated: false, any: false, expr, pattern, escape_char: None }
|
||||
| Expr::ILike { negated: false, any: false, expr, pattern, escape_char: None } => {
|
||||
expr_contains_column_reference(expr) && like_pattern_matches_all(pattern)
|
||||
}
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn or_contains_conjunctive_branch(expr: &Expr) -> bool {
|
||||
match strip_nested_expr(expr) {
|
||||
Expr::BinaryOp { left, op: BinaryOperator::Or, right } => {
|
||||
or_contains_conjunctive_branch(left) || or_contains_conjunctive_branch(right)
|
||||
}
|
||||
Expr::BinaryOp { op: BinaryOperator::And, .. } => true,
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn expr_contains_column_reference(expr: &Expr) -> bool {
|
||||
visit_expressions(expr, |expr| {
|
||||
let is_column = match expr {
|
||||
Expr::Identifier(identifier) => !is_sql_value_keyword(&identifier.value),
|
||||
Expr::CompoundIdentifier(_) => true,
|
||||
_ => false,
|
||||
};
|
||||
if is_column {
|
||||
ControlFlow::Break(())
|
||||
} else {
|
||||
ControlFlow::Continue(())
|
||||
}
|
||||
})
|
||||
.is_break()
|
||||
}
|
||||
|
||||
fn is_sql_value_keyword(value: &str) -> bool {
|
||||
matches!(
|
||||
value.to_ascii_lowercase().as_str(),
|
||||
"current_catalog"
|
||||
| "current_date"
|
||||
| "current_role"
|
||||
| "current_schema"
|
||||
| "current_time"
|
||||
| "current_timestamp"
|
||||
| "current_user"
|
||||
| "localtime"
|
||||
| "localtimestamp"
|
||||
| "session_user"
|
||||
| "system_user"
|
||||
| "user"
|
||||
)
|
||||
}
|
||||
|
||||
fn expr_contains_subquery(expr: &Expr) -> bool {
|
||||
visit_expressions(expr, |expr| {
|
||||
if matches!(expr, Expr::InSubquery { .. } | Expr::Exists { .. } | Expr::Subquery(_)) {
|
||||
ControlFlow::Break(())
|
||||
} else {
|
||||
ControlFlow::Continue(())
|
||||
}
|
||||
})
|
||||
.is_break()
|
||||
}
|
||||
|
||||
fn or_contains_complementary_null_checks(expr: &Expr) -> bool {
|
||||
let mut checks = Vec::new();
|
||||
collect_or_null_checks(expr, &mut checks);
|
||||
checks.iter().enumerate().any(|(index, (candidate, negated))| {
|
||||
checks[index + 1..].iter().any(|(other, other_negated)| candidate == other && negated != other_negated)
|
||||
})
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, PartialEq, Eq)]
|
||||
enum PredicateComparisonOperator {
|
||||
Eq,
|
||||
NotEq,
|
||||
Gt,
|
||||
GtEq,
|
||||
Lt,
|
||||
LtEq,
|
||||
}
|
||||
|
||||
struct PredicateComparisonCheck<'a> {
|
||||
left: &'a Expr,
|
||||
right: &'a Expr,
|
||||
operator: PredicateComparisonOperator,
|
||||
}
|
||||
|
||||
fn or_contains_complementary_comparisons(expr: &Expr) -> bool {
|
||||
let mut checks = Vec::new();
|
||||
collect_or_comparison_checks(expr, &mut checks);
|
||||
checks.iter().enumerate().any(|(index, candidate)| {
|
||||
checks[index + 1..].iter().any(|other| {
|
||||
candidate.left == other.left
|
||||
&& candidate.right == other.right
|
||||
&& comparison_operators_are_complementary(candidate.operator, other.operator)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
fn collect_or_comparison_checks<'a>(expr: &'a Expr, checks: &mut Vec<PredicateComparisonCheck<'a>>) {
|
||||
let expr = strip_nested_expr(expr);
|
||||
if let Expr::BinaryOp { left, op: BinaryOperator::Or, right } = expr {
|
||||
collect_or_comparison_checks(left, checks);
|
||||
collect_or_comparison_checks(right, checks);
|
||||
} else if let Some(check) = comparison_check(expr, false) {
|
||||
checks.push(check);
|
||||
}
|
||||
}
|
||||
|
||||
fn comparison_check(expr: &Expr, outer_negated: bool) -> Option<PredicateComparisonCheck<'_>> {
|
||||
let expr = strip_nested_expr(expr);
|
||||
if let Expr::UnaryOp { op: UnaryOperator::Not, expr } = expr {
|
||||
return comparison_check(expr, !outer_negated);
|
||||
}
|
||||
let Expr::BinaryOp { left, op, right } = expr else {
|
||||
return None;
|
||||
};
|
||||
if !expr_contains_column_reference(expr) {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut operator = predicate_comparison_operator(op)?;
|
||||
if outer_negated {
|
||||
operator = complementary_comparison_operator(operator);
|
||||
}
|
||||
let (left, right) = (strip_nested_expr(left), strip_nested_expr(right));
|
||||
if left <= right {
|
||||
Some(PredicateComparisonCheck { left, right, operator })
|
||||
} else {
|
||||
Some(PredicateComparisonCheck { left: right, right: left, operator: reverse_comparison_operator(operator) })
|
||||
}
|
||||
}
|
||||
|
||||
fn predicate_comparison_operator(operator: &BinaryOperator) -> Option<PredicateComparisonOperator> {
|
||||
match operator {
|
||||
BinaryOperator::Eq | BinaryOperator::Spaceship => Some(PredicateComparisonOperator::Eq),
|
||||
BinaryOperator::NotEq => Some(PredicateComparisonOperator::NotEq),
|
||||
BinaryOperator::Gt => Some(PredicateComparisonOperator::Gt),
|
||||
BinaryOperator::GtEq => Some(PredicateComparisonOperator::GtEq),
|
||||
BinaryOperator::Lt => Some(PredicateComparisonOperator::Lt),
|
||||
BinaryOperator::LtEq => Some(PredicateComparisonOperator::LtEq),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn complementary_comparison_operator(operator: PredicateComparisonOperator) -> PredicateComparisonOperator {
|
||||
match operator {
|
||||
PredicateComparisonOperator::Eq => PredicateComparisonOperator::NotEq,
|
||||
PredicateComparisonOperator::NotEq => PredicateComparisonOperator::Eq,
|
||||
PredicateComparisonOperator::Gt => PredicateComparisonOperator::LtEq,
|
||||
PredicateComparisonOperator::GtEq => PredicateComparisonOperator::Lt,
|
||||
PredicateComparisonOperator::Lt => PredicateComparisonOperator::GtEq,
|
||||
PredicateComparisonOperator::LtEq => PredicateComparisonOperator::Gt,
|
||||
}
|
||||
}
|
||||
|
||||
fn reverse_comparison_operator(operator: PredicateComparisonOperator) -> PredicateComparisonOperator {
|
||||
match operator {
|
||||
PredicateComparisonOperator::Eq | PredicateComparisonOperator::NotEq => operator,
|
||||
PredicateComparisonOperator::Gt => PredicateComparisonOperator::Lt,
|
||||
PredicateComparisonOperator::GtEq => PredicateComparisonOperator::LtEq,
|
||||
PredicateComparisonOperator::Lt => PredicateComparisonOperator::Gt,
|
||||
PredicateComparisonOperator::LtEq => PredicateComparisonOperator::GtEq,
|
||||
}
|
||||
}
|
||||
|
||||
fn comparison_operators_are_complementary(
|
||||
left: PredicateComparisonOperator,
|
||||
right: PredicateComparisonOperator,
|
||||
) -> bool {
|
||||
complementary_comparison_operator(left) == right
|
||||
}
|
||||
|
||||
struct PredicateInListCheck<'a> {
|
||||
expr: &'a Expr,
|
||||
list: Vec<&'a Expr>,
|
||||
negated: bool,
|
||||
}
|
||||
|
||||
fn or_contains_complementary_in_lists(expr: &Expr) -> bool {
|
||||
let mut checks = Vec::new();
|
||||
collect_or_in_list_checks(expr, &mut checks);
|
||||
checks.iter().enumerate().any(|(index, candidate)| {
|
||||
checks[index + 1..].iter().any(|other| {
|
||||
candidate.expr == other.expr && candidate.list == other.list && candidate.negated != other.negated
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
fn collect_or_in_list_checks<'a>(expr: &'a Expr, checks: &mut Vec<PredicateInListCheck<'a>>) {
|
||||
let expr = strip_nested_expr(expr);
|
||||
if let Expr::BinaryOp { left, op: BinaryOperator::Or, right } = expr {
|
||||
collect_or_in_list_checks(left, checks);
|
||||
collect_or_in_list_checks(right, checks);
|
||||
} else if let Some(check) = in_list_check(expr, false) {
|
||||
checks.push(check);
|
||||
}
|
||||
}
|
||||
|
||||
fn in_list_check(expr: &Expr, outer_negated: bool) -> Option<PredicateInListCheck<'_>> {
|
||||
match strip_nested_expr(expr) {
|
||||
Expr::UnaryOp { op: UnaryOperator::Not, expr } => in_list_check(expr, !outer_negated),
|
||||
Expr::InList { expr, list, negated } if expr_contains_column_reference(expr) && !list.is_empty() => {
|
||||
let mut normalized_list = list.iter().map(strip_nested_expr).collect::<Vec<_>>();
|
||||
if normalized_list.iter().all(|item| matches!(strip_nested_expr(item), Expr::Value(_))) {
|
||||
normalized_list.sort_unstable();
|
||||
normalized_list.dedup();
|
||||
}
|
||||
Some(PredicateInListCheck {
|
||||
expr: strip_nested_expr(expr),
|
||||
list: normalized_list,
|
||||
negated: *negated != outer_negated,
|
||||
})
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
struct PredicateBetweenCheck<'a> {
|
||||
expr: &'a Expr,
|
||||
low: &'a Expr,
|
||||
high: &'a Expr,
|
||||
negated: bool,
|
||||
}
|
||||
|
||||
fn or_contains_complementary_between_checks(expr: &Expr) -> bool {
|
||||
let mut checks = Vec::new();
|
||||
collect_or_between_checks(expr, &mut checks);
|
||||
checks.iter().enumerate().any(|(index, candidate)| {
|
||||
checks[index + 1..].iter().any(|other| {
|
||||
candidate.expr == other.expr
|
||||
&& candidate.low == other.low
|
||||
&& candidate.high == other.high
|
||||
&& candidate.negated != other.negated
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
fn collect_or_between_checks<'a>(expr: &'a Expr, checks: &mut Vec<PredicateBetweenCheck<'a>>) {
|
||||
let expr = strip_nested_expr(expr);
|
||||
if let Expr::BinaryOp { left, op: BinaryOperator::Or, right } = expr {
|
||||
collect_or_between_checks(left, checks);
|
||||
collect_or_between_checks(right, checks);
|
||||
} else if let Some(check) = between_check(expr, false) {
|
||||
checks.push(check);
|
||||
}
|
||||
}
|
||||
|
||||
fn between_check(expr: &Expr, outer_negated: bool) -> Option<PredicateBetweenCheck<'_>> {
|
||||
match strip_nested_expr(expr) {
|
||||
Expr::UnaryOp { op: UnaryOperator::Not, expr } => between_check(expr, !outer_negated),
|
||||
Expr::Between { expr, negated, low, high } if expr_contains_column_reference(expr) => {
|
||||
Some(PredicateBetweenCheck {
|
||||
expr: strip_nested_expr(expr),
|
||||
low: strip_nested_expr(low),
|
||||
high: strip_nested_expr(high),
|
||||
negated: *negated != outer_negated,
|
||||
})
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn like_pattern_matches_all(expr: &Expr) -> bool {
|
||||
matches!(strip_nested_expr(expr), Expr::Value(value) if matches!(&value.value, Value::SingleQuotedString(pattern) if !pattern.is_empty() && pattern.chars().all(|character| character == '%')))
|
||||
}
|
||||
|
||||
fn collect_or_null_checks<'a>(expr: &'a Expr, checks: &mut Vec<(&'a Expr, bool)>) {
|
||||
let expr = strip_nested_expr(expr);
|
||||
if let Expr::BinaryOp { left, op: BinaryOperator::Or, right } = expr {
|
||||
collect_or_null_checks(left, checks);
|
||||
collect_or_null_checks(right, checks);
|
||||
} else if let Some(check) = null_check(expr) {
|
||||
checks.push(check);
|
||||
}
|
||||
}
|
||||
|
||||
fn null_check(expr: &Expr) -> Option<(&Expr, bool)> {
|
||||
null_check_with_negation(expr, false)
|
||||
}
|
||||
|
||||
fn null_check_with_negation(expr: &Expr, outer_negated: bool) -> Option<(&Expr, bool)> {
|
||||
match strip_nested_expr(expr) {
|
||||
Expr::UnaryOp { op: UnaryOperator::Not, expr } => null_check_with_negation(expr, !outer_negated),
|
||||
Expr::IsNull(expr) => Some((strip_nested_expr(expr), outer_negated)),
|
||||
Expr::IsNotNull(expr) => Some((strip_nested_expr(expr), !outer_negated)),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn strip_nested_expr(mut expr: &Expr) -> &Expr {
|
||||
while let Expr::Nested(inner) = expr {
|
||||
expr = inner;
|
||||
}
|
||||
expr
|
||||
}
|
||||
|
||||
fn predicate_is_obviously_false(expr: &Expr) -> bool {
|
||||
match expr {
|
||||
Expr::Nested(expr) => predicate_is_obviously_false(expr),
|
||||
Expr::Value(value) => value_is_falsy(&value.value),
|
||||
Expr::UnaryOp { op: UnaryOperator::Not, expr } => predicate_is_obviously_unbounded(expr),
|
||||
Expr::BinaryOp { left, op: BinaryOperator::And, right } => {
|
||||
predicate_is_obviously_false(left) || predicate_is_obviously_false(right)
|
||||
}
|
||||
Expr::BinaryOp { left, op: BinaryOperator::Or, right } => {
|
||||
predicate_is_obviously_false(left) && predicate_is_obviously_false(right)
|
||||
}
|
||||
Expr::BinaryOp { left, op, right } if is_comparison_operator(op) => {
|
||||
constant_comparison_truth(left, op, right).is_some_and(|result| !result)
|
||||
|| (left == right && matches!(op, BinaryOperator::NotEq | BinaryOperator::Gt | BinaryOperator::Lt))
|
||||
}
|
||||
Expr::IsDistinctFrom(left, right) => strip_nested_expr(left) == strip_nested_expr(right),
|
||||
Expr::IsFalse(expr) | Expr::IsNotTrue(expr) => predicate_is_obviously_unbounded(expr),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn is_comparison_operator(operator: &BinaryOperator) -> bool {
|
||||
matches!(
|
||||
operator,
|
||||
BinaryOperator::Eq
|
||||
| BinaryOperator::NotEq
|
||||
| BinaryOperator::Gt
|
||||
| BinaryOperator::Lt
|
||||
| BinaryOperator::GtEq
|
||||
| BinaryOperator::LtEq
|
||||
| BinaryOperator::Spaceship
|
||||
)
|
||||
}
|
||||
|
||||
fn constant_comparison_truth(left: &Expr, operator: &BinaryOperator, right: &Expr) -> Option<bool> {
|
||||
let (Expr::Value(left), Expr::Value(right)) = (left, right) else {
|
||||
return None;
|
||||
};
|
||||
match operator {
|
||||
BinaryOperator::Eq | BinaryOperator::Spaceship => Some(left.value == right.value),
|
||||
BinaryOperator::NotEq => Some(left.value != right.value),
|
||||
BinaryOperator::Gt | BinaryOperator::Lt | BinaryOperator::GtEq | BinaryOperator::LtEq => {
|
||||
let (Value::Number(left, _), Value::Number(right, _)) = (&left.value, &right.value) else {
|
||||
return None;
|
||||
};
|
||||
let left = left.parse::<f64>().ok()?;
|
||||
let right = right.parse::<f64>().ok()?;
|
||||
Some(match operator {
|
||||
BinaryOperator::Gt => left > right,
|
||||
BinaryOperator::Lt => left < right,
|
||||
BinaryOperator::GtEq => left >= right,
|
||||
BinaryOperator::LtEq => left <= right,
|
||||
_ => unreachable!(),
|
||||
})
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn value_is_truthy(value: &Value) -> bool {
|
||||
match value {
|
||||
Value::Boolean(value) => *value,
|
||||
Value::Number(value, _) => value.parse::<f64>().is_ok_and(|number| number != 0.0),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn value_is_falsy(value: &Value) -> bool {
|
||||
match value {
|
||||
Value::Boolean(value) => !*value,
|
||||
Value::Number(value, _) => value.parse::<f64>().is_ok_and(|number| number == 0.0),
|
||||
Value::Null => true,
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
const SIDE_EFFECT_SELECT_FUNCTIONS: &[&str] = &[
|
||||
"lo_create",
|
||||
"lo_import",
|
||||
"lo_unlink",
|
||||
"nextval",
|
||||
"pg_advisory_lock",
|
||||
"pg_advisory_unlock",
|
||||
"pg_advisory_unlock_all",
|
||||
"pg_advisory_xact_lock",
|
||||
"pg_cancel_backend",
|
||||
"pg_reload_conf",
|
||||
"pg_terminate_backend",
|
||||
"pg_try_advisory_lock",
|
||||
"pg_try_advisory_xact_lock",
|
||||
"setval",
|
||||
];
|
||||
|
||||
fn query_is_write_capable(query: &Query, detect_select_into: bool) -> bool {
|
||||
query
|
||||
.with
|
||||
.as_ref()
|
||||
.is_some_and(|with| with.cte_tables.iter().any(|cte| query_is_write_capable(&cte.query, detect_select_into)))
|
||||
|| set_expr_is_write_capable(&query.body, detect_select_into)
|
||||
|| !query.locks.is_empty()
|
||||
|| query_calls_known_side_effect_function(query)
|
||||
}
|
||||
|
||||
fn set_expr_is_write_capable(expr: &SetExpr, detect_select_into: bool) -> bool {
|
||||
match expr {
|
||||
SetExpr::Select(select) => detect_select_into && select.into.is_some(),
|
||||
SetExpr::Query(query) => query_is_write_capable(query, detect_select_into),
|
||||
SetExpr::SetOperation { left, right, .. } => {
|
||||
set_expr_is_write_capable(left, detect_select_into) || set_expr_is_write_capable(right, detect_select_into)
|
||||
}
|
||||
SetExpr::Insert(_) | SetExpr::Update(_) | SetExpr::Delete(_) | SetExpr::Merge(_) => true,
|
||||
SetExpr::Values(_) | SetExpr::Table(_) => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn query_is_dangerous(query: &Query, detect_select_into: bool) -> bool {
|
||||
query
|
||||
.with
|
||||
.as_ref()
|
||||
.is_some_and(|with| with.cte_tables.iter().any(|cte| query_is_dangerous(&cte.query, detect_select_into)))
|
||||
|| set_expr_is_dangerous(&query.body, detect_select_into)
|
||||
|| !query.locks.is_empty()
|
||||
|| query_calls_known_side_effect_function(query)
|
||||
}
|
||||
|
||||
fn set_expr_is_dangerous(expr: &SetExpr, detect_select_into: bool) -> bool {
|
||||
match expr {
|
||||
SetExpr::Select(select) => detect_select_into && select.into.is_some(),
|
||||
SetExpr::Query(query) => query_is_dangerous(query, detect_select_into),
|
||||
SetExpr::SetOperation { left, right, .. } => {
|
||||
set_expr_is_dangerous(left, detect_select_into) || set_expr_is_dangerous(right, detect_select_into)
|
||||
}
|
||||
SetExpr::Insert(statement)
|
||||
| SetExpr::Update(statement)
|
||||
| SetExpr::Delete(statement)
|
||||
| SetExpr::Merge(statement) => statement_is_dangerous(statement, detect_select_into),
|
||||
SetExpr::Values(_) | SetExpr::Table(_) => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn query_calls_known_side_effect_function(query: &Query) -> bool {
|
||||
visit_expressions(query, |expr| {
|
||||
let is_side_effect = if let Expr::Function(function) = expr {
|
||||
function.name.0.last().and_then(|part| part.as_ident()).is_some_and(|name| {
|
||||
SIDE_EFFECT_SELECT_FUNCTIONS.iter().any(|candidate| name.value.eq_ignore_ascii_case(candidate))
|
||||
})
|
||||
} else {
|
||||
false
|
||||
};
|
||||
if is_side_effect {
|
||||
ControlFlow::Break(())
|
||||
} else {
|
||||
ControlFlow::Continue(())
|
||||
}
|
||||
})
|
||||
.is_break()
|
||||
}
|
||||
|
||||
/// Classify SQL risk using sqlparser AST analysis.
|
||||
///
|
||||
/// If parsing fails (non-standard SQL, non-SQL databases), falls back to
|
||||
@@ -162,6 +678,100 @@ pub fn classify_sql_risk_for_database(sql: &str, database_type: DatabaseType) ->
|
||||
classify_sql_risk_with_database(sql, normalized, Some(database_type))
|
||||
}
|
||||
|
||||
/// Return whether MCP must require the central dangerous-operation permission.
|
||||
/// Parse failures fail closed for writes. Safe-write mode permits plain INSERT
|
||||
/// and single-table UPDATE/DELETE statements with an effective predicate;
|
||||
/// broader or opaque mutations require central high-risk permission.
|
||||
pub fn is_dangerous_sql_for_database(sql: &str, database_type: DatabaseType) -> bool {
|
||||
let database_type_name = format!("{database_type:?}");
|
||||
let normalized = normalize_dialect(&database_type_name);
|
||||
let parser_dialect = resolve_dialect(normalized);
|
||||
let detect_select_into = supports_select_into_table_creation(database_type);
|
||||
let has_locking_clause = sql_contains_top_level_locking_clause(sql, parser_dialect.as_ref());
|
||||
// PostgreSQL-family and SQL Server SELECT INTO forms are represented in
|
||||
// the AST. Their text fallback also matches ordinary INSERT INTO, so only
|
||||
// use it for dialect-specific writes that the selected AST cannot express
|
||||
// (notably MySQL INTO OUTFILE/DUMPFILE and executable comments).
|
||||
let has_unparsed_dialect_specific_write =
|
||||
!detect_select_into && crate::query_execution_sql::has_dialect_specific_write(sql, database_type);
|
||||
|
||||
match Parser::parse_sql(parser_dialect.as_ref(), sql) {
|
||||
Ok(statements) if !statements.is_empty() => {
|
||||
has_locking_clause
|
||||
|| has_unparsed_dialect_specific_write
|
||||
|| statements.iter().any(|statement| statement_is_dangerous(statement, detect_select_into))
|
||||
}
|
||||
_ => has_locking_clause || crate::query_execution_sql::is_write_sql_for_database(sql, database_type),
|
||||
}
|
||||
}
|
||||
|
||||
/// MCP requests must select their database through the explicit request scope.
|
||||
/// A `USE` statement mutates pooled/session state and could redirect later SQL,
|
||||
/// so it is forbidden independently of read/write and high-risk permissions.
|
||||
pub fn mcp_sql_has_forbidden_database_switch(sql: &str, database_type: DatabaseType) -> bool {
|
||||
let database_type_name = format!("{database_type:?}");
|
||||
let normalized = normalize_dialect(&database_type_name);
|
||||
let dialect = resolve_dialect(normalized);
|
||||
if sql_has_use_statement(sql, dialect.as_ref()) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if matches!(
|
||||
database_type,
|
||||
DatabaseType::Mysql
|
||||
| DatabaseType::Doris
|
||||
| DatabaseType::StarRocks
|
||||
| DatabaseType::ManticoreSearch
|
||||
| DatabaseType::Goldendb
|
||||
) {
|
||||
let executable_comments_expanded = crate::query_execution_sql::strip_sql_comments(sql);
|
||||
return sql_has_use_statement(&executable_comments_expanded, dialect.as_ref());
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
fn sql_has_use_statement(sql: &str, dialect: &dyn sqlparser::dialect::Dialect) -> bool {
|
||||
if let Ok(statements) = Parser::parse_sql(dialect, sql) {
|
||||
return statements.iter().any(|statement| matches!(statement, Statement::Use(_)));
|
||||
}
|
||||
|
||||
let Ok(tokens) = Tokenizer::new(dialect, sql).tokenize() else {
|
||||
return false;
|
||||
};
|
||||
let mut statement_start = true;
|
||||
for token in tokens {
|
||||
match token {
|
||||
Token::Whitespace(_) => {}
|
||||
Token::SemiColon => statement_start = true,
|
||||
Token::Word(word) if statement_start => {
|
||||
if word.value.eq_ignore_ascii_case("use") {
|
||||
return true;
|
||||
}
|
||||
statement_start = false;
|
||||
}
|
||||
Token::EOF => {}
|
||||
_ if statement_start => statement_start = false,
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
fn supports_select_into_table_creation(database_type: DatabaseType) -> bool {
|
||||
matches!(
|
||||
database_type,
|
||||
DatabaseType::Postgres
|
||||
| DatabaseType::Redshift
|
||||
| DatabaseType::Gaussdb
|
||||
| DatabaseType::OpenGauss
|
||||
| DatabaseType::Kingbase
|
||||
| DatabaseType::Highgo
|
||||
| DatabaseType::Vastbase
|
||||
| DatabaseType::Kwdb
|
||||
| DatabaseType::SqlServer
|
||||
)
|
||||
}
|
||||
|
||||
fn classify_sql_risk_with_database(
|
||||
sql: &str,
|
||||
normalized_dialect: &str,
|
||||
@@ -169,6 +779,7 @@ fn classify_sql_risk_with_database(
|
||||
) -> Result<SqlRisk, String> {
|
||||
let parser_dialect = resolve_dialect(normalized_dialect);
|
||||
let detect_select_into = database_type.is_none();
|
||||
let has_locking_clause = sql_contains_top_level_locking_clause(sql, parser_dialect.as_ref());
|
||||
let has_dialect_specific_write = database_type
|
||||
.is_some_and(|database_type| crate::query_execution_sql::has_dialect_specific_write(sql, database_type));
|
||||
|
||||
@@ -181,7 +792,7 @@ fn classify_sql_risk_with_database(
|
||||
max_risk = risk;
|
||||
}
|
||||
}
|
||||
if max_risk == SqlRisk::ReadOnly && has_dialect_specific_write {
|
||||
if max_risk == SqlRisk::ReadOnly && (has_dialect_specific_write || has_locking_clause) {
|
||||
Ok(SqlRisk::Write)
|
||||
} else {
|
||||
Ok(max_risk)
|
||||
@@ -193,7 +804,7 @@ fn classify_sql_risk_with_database(
|
||||
|| crate::query_execution_sql::is_write_sql(sql),
|
||||
|database_type| crate::query_execution_sql::is_write_sql_for_database(sql, database_type),
|
||||
);
|
||||
if is_write {
|
||||
if is_write || has_locking_clause {
|
||||
Ok(SqlRisk::Write)
|
||||
} else {
|
||||
Ok(SqlRisk::ReadOnly)
|
||||
@@ -202,6 +813,34 @@ fn classify_sql_risk_with_database(
|
||||
}
|
||||
}
|
||||
|
||||
fn sql_contains_top_level_locking_clause(sql: &str, dialect: &dyn sqlparser::dialect::Dialect) -> bool {
|
||||
let Ok(tokens) = Tokenizer::new(dialect, sql).tokenize() else {
|
||||
return false;
|
||||
};
|
||||
let mut depth: usize = 0;
|
||||
let mut words = Vec::new();
|
||||
for token in tokens {
|
||||
match token {
|
||||
Token::LParen => depth += 1,
|
||||
Token::RParen => depth = depth.saturating_sub(1),
|
||||
token if depth == 0 => {
|
||||
let word = token.to_string();
|
||||
if word.chars().all(|character| character.is_ascii_alphabetic() || character == '_') {
|
||||
words.push(word.to_ascii_uppercase());
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
words.windows(2).any(|window| matches!(window, [r#for, lock] if r#for == "FOR" && matches!(lock.as_str(), "UPDATE" | "SHARE")))
|
||||
|| words.windows(3).any(|window| {
|
||||
matches!(window, [r#for, key, share] if r#for == "FOR" && key == "KEY" && share == "SHARE")
|
||||
})
|
||||
|| words.windows(4).any(|window| {
|
||||
matches!(window, [r#for, no, key, update] if r#for == "FOR" && no == "NO" && key == "KEY" && update == "UPDATE")
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -226,6 +865,42 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_writable_ctes_recursively() {
|
||||
for sql in [
|
||||
"WITH inserted AS (INSERT INTO users (id) VALUES (1) RETURNING id) SELECT * FROM inserted",
|
||||
"WITH updated AS (UPDATE users SET active = true WHERE id = 1 RETURNING id) SELECT * FROM updated",
|
||||
"WITH deleted AS (DELETE FROM users WHERE id = 1 RETURNING id) SELECT * FROM deleted",
|
||||
"WITH merged AS (MERGE INTO users USING staged_users ON users.id = staged_users.id WHEN MATCHED THEN UPDATE SET active = true RETURNING users.id) SELECT * FROM merged",
|
||||
"WITH outer_cte AS (WITH deleted AS (DELETE FROM users WHERE id = 1 RETURNING id) SELECT * FROM deleted) SELECT * FROM outer_cte",
|
||||
] {
|
||||
assert_eq!(classify_sql_risk(sql, "postgres").unwrap(), SqlRisk::Write, "expected writable CTE: {sql}");
|
||||
assert!(
|
||||
crate::query_execution_sql::is_write_sql_for_database(sql, DatabaseType::Postgres),
|
||||
"expected writable CTE to trip read-only enforcement: {sql}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn writable_cte_danger_tracks_the_nested_mutation() {
|
||||
for sql in [
|
||||
"WITH updated AS (UPDATE users SET active = true WHERE id = 1 RETURNING id) SELECT * FROM updated",
|
||||
"WITH deleted AS (DELETE FROM users WHERE id = 1 RETURNING id) SELECT * FROM deleted",
|
||||
"WITH outer_cte AS (WITH deleted AS (DELETE FROM users WHERE id = 1 RETURNING id) SELECT * FROM deleted) SELECT * FROM outer_cte",
|
||||
] {
|
||||
assert!(!is_dangerous_sql_for_database(sql, DatabaseType::Postgres), "expected guarded write: {sql}");
|
||||
}
|
||||
|
||||
for sql in [
|
||||
"WITH updated AS (UPDATE users SET active = true RETURNING id) SELECT * FROM updated",
|
||||
"WITH deleted AS (DELETE FROM users RETURNING id) SELECT * FROM deleted",
|
||||
"WITH outer_cte AS (WITH deleted AS (DELETE FROM users RETURNING id) SELECT * FROM deleted) SELECT * FROM outer_cte",
|
||||
] {
|
||||
assert!(is_dangerous_sql_for_database(sql, DatabaseType::Postgres), "expected dangerous write: {sql}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_write_statements() {
|
||||
assert_eq!(classify_sql_risk("INSERT INTO users VALUES (1)", "postgres").unwrap(), SqlRisk::Write);
|
||||
@@ -240,6 +915,45 @@ mod tests {
|
||||
assert_eq!(classify_sql_risk("/*! DELETE FROM users */", "mysql").unwrap(), SqlRisk::Write);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_forbids_persistent_database_switching_in_every_permission_mode() {
|
||||
for (sql, database_type) in [
|
||||
("USE reporting", DatabaseType::Mysql),
|
||||
("-- target database\nUSE reporting", DatabaseType::Mysql),
|
||||
("SELECT 1; USE reporting", DatabaseType::Mysql),
|
||||
("USE [reporting]", DatabaseType::SqlServer),
|
||||
("USE DATABASE reporting", DatabaseType::Snowflake),
|
||||
("/*!50000 USE reporting */", DatabaseType::Mysql),
|
||||
] {
|
||||
assert!(mcp_sql_has_forbidden_database_switch(sql, database_type), "expected blocked SQL: {sql}");
|
||||
}
|
||||
|
||||
for sql in ["SELECT use FROM feature_flags", "SELECT 'USE reporting'", "SELECT 1"] {
|
||||
assert!(!mcp_sql_has_forbidden_database_switch(sql, DatabaseType::Mysql), "expected allowed SQL: {sql}");
|
||||
}
|
||||
assert!(!mcp_sql_has_forbidden_database_switch("/*!50000 USE reporting */", DatabaseType::Postgres));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_known_side_effect_selects_and_copy_as_writes() {
|
||||
for sql in [
|
||||
"SELECT setval('user_id_seq', 42)",
|
||||
"SELECT nextval('user_id_seq')",
|
||||
"SELECT pg_terminate_backend(42)",
|
||||
"SELECT * FROM users FOR UPDATE",
|
||||
"SELECT * FROM users FOR KEY SHARE",
|
||||
"COPY users TO '/tmp/users.csv'",
|
||||
"COPY (SELECT * FROM users) TO PROGRAM 'cat > /tmp/users.csv'",
|
||||
] {
|
||||
assert_eq!(
|
||||
classify_sql_risk_for_database(sql, DatabaseType::Postgres).unwrap(),
|
||||
SqlRisk::Write,
|
||||
"expected write-capable SQL: {sql}"
|
||||
);
|
||||
assert!(is_dangerous_sql_for_database(sql, DatabaseType::Postgres), "expected high-risk SQL: {sql}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_dialect_specific_select_into_as_write() {
|
||||
for sql in [
|
||||
@@ -303,6 +1017,194 @@ mod tests {
|
||||
assert_eq!(classify_sql_risk("TRUNCATE TABLE users", "postgres").unwrap(), SqlRisk::Ddl);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn high_risk_sql_requires_central_permission_for_unbounded_changes() {
|
||||
assert!(is_dangerous_sql_for_database("TRUNCATE TABLE users", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database("UPDATE users SET active = 0", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users WHERE 1 = 1", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database("UPDATE users SET active = 0 WHERE TRUE", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users WHERE id = id", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE lower(email) = lower(email)",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users WHERE NOT (1 = 0)", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database("UPDATE users SET active = 0 WHERE 2 > 1", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE id IS NULL OR id IS NOT NULL",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE id IS NULL OR NOT (((id IS NULL)))",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE id IS NOT NULL OR NOT (((id IS NOT NULL)))",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users WHERE id = 1 OR id <> 1", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users WHERE id != 1 OR 1 = id", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE status = 'disabled' OR status != 'disabled'",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE (id = 1 OR status = 'disabled') OR 1 != id OR id IS NULL",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users WHERE id > 1 OR id <= 1", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users WHERE 1 >= id OR id > 1", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users WHERE id >= 1 OR 1 > id", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE id IN (1) OR id NOT IN (1) OR id IS NULL",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE id IN (1, 2) OR id NOT IN (2, 1) OR id IS NULL",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE id BETWEEN 1 AND 2 OR id NOT BETWEEN 1 AND 2 OR id IS NULL",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE id = 1 OR (id <> 1 AND TRUE) OR id IS NULL",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE id IS NOT DISTINCT FROM id",
|
||||
DatabaseType::Postgres
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users WHERE id <=> id", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"UPDATE users SET active = 0 WHERE name LIKE '%' OR name IS NULL",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"UPDATE users SET active = 0 WHERE name LIKE '%%' OR name IS NULL",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE (id IS NULL OR status = 'disabled') OR id IS NOT NULL",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database("UPDATE users SET active = 0 WHERE abs(1) = 1", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users WHERE lower('A') = 'a'", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users WHERE coalesce(NULL, 1) = 1", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users WHERE LOWER(_utf8mb4'A') = 'a'", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE EXTRACT(YEAR FROM DATE '2026-01-01') = 2026",
|
||||
DatabaseType::Postgres
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE DATE '2026-01-01' < CURRENT_DATE",
|
||||
DatabaseType::Postgres
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database("DELETE FROM users WHERE USER = CURRENT_USER", DatabaseType::Postgres));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE id IN (SELECT id FROM archived_users)",
|
||||
DatabaseType::Postgres
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE EXISTS (SELECT 1 FROM archived_users)",
|
||||
DatabaseType::Postgres
|
||||
));
|
||||
assert!(!is_dangerous_sql_for_database("DELETE FROM users WHERE id = 1", DatabaseType::Mysql));
|
||||
assert!(!is_dangerous_sql_for_database("UPDATE users SET active = 0 WHERE id = 1", DatabaseType::Mysql));
|
||||
assert!(!is_dangerous_sql_for_database("UPDATE users SET active = 0 WHERE abs(id) = 1", DatabaseType::Mysql));
|
||||
assert!(!is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE lower(email) = 'disabled@example.com'",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(!is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE EXTRACT(YEAR FROM created_at) = 2026",
|
||||
DatabaseType::Postgres
|
||||
));
|
||||
assert!(!is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE id IS NULL OR status IS NOT NULL",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(!is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE (id IS NULL OR NOT (((id IS NULL)))) AND tenant_id = 1",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(!is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE (id = 1 OR id <> 1) AND tenant_id = 1",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(!is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE status = 'pending' OR status <> 'disabled'",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(!is_dangerous_sql_for_database("DELETE FROM users WHERE id IN (1) OR id IN (2)", DatabaseType::Mysql));
|
||||
assert!(!is_dangerous_sql_for_database(
|
||||
"DELETE FROM users WHERE id BETWEEN 1 AND 2 OR id BETWEEN 4 AND 5",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(!is_dangerous_sql_for_database(
|
||||
"UPDATE users SET active = 0 WHERE name LIKE 'admin%'",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(!is_dangerous_sql_for_database(
|
||||
"UPDATE users SET active = 0 WHERE status = 'inactive' AND tenant_id = 1",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"UPDATE users JOIN accounts ON accounts.id = users.account_id SET users.active = 0 WHERE users.id = 1",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"UPDATE users SET active = false FROM accounts WHERE users.account_id = accounts.id",
|
||||
DatabaseType::Postgres
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"DELETE users FROM users JOIN accounts ON accounts.id = users.account_id WHERE users.id = 1",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database("REPLACE INTO users (id) VALUES (1)", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"INSERT INTO users (id) VALUES (1) ON DUPLICATE KEY UPDATE active = 1",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"INSERT INTO users (id) VALUES (1) ON CONFLICT (id) DO UPDATE SET active = true",
|
||||
DatabaseType::Postgres
|
||||
));
|
||||
assert!(!is_dangerous_sql_for_database("INSERT INTO users (id) VALUES (1)", DatabaseType::Mysql));
|
||||
assert!(is_dangerous_sql_for_database(
|
||||
"INSERT INTO users (id) SELECT id FROM staged_users",
|
||||
DatabaseType::Mysql
|
||||
));
|
||||
assert!(!is_dangerous_sql_for_database(
|
||||
"INSERT INTO users (id) VALUES (1) ON CONFLICT (id) DO NOTHING",
|
||||
DatabaseType::Postgres
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn select_into_requires_central_high_risk_permission_for_supported_databases() {
|
||||
for database_type in [
|
||||
DatabaseType::Postgres,
|
||||
DatabaseType::Redshift,
|
||||
DatabaseType::Gaussdb,
|
||||
DatabaseType::OpenGauss,
|
||||
DatabaseType::Kingbase,
|
||||
DatabaseType::Highgo,
|
||||
DatabaseType::Vastbase,
|
||||
DatabaseType::Kwdb,
|
||||
] {
|
||||
assert!(
|
||||
is_dangerous_sql_for_database("SELECT * INTO copied_users FROM users", database_type),
|
||||
"expected PostgreSQL-family SELECT INTO to require high-risk permission for {database_type:?}"
|
||||
);
|
||||
}
|
||||
assert!(is_dangerous_sql_for_database("SELECT * INTO #copied_users FROM users", DatabaseType::SqlServer));
|
||||
assert!(is_dangerous_sql_for_database("SELECT 1 INTO OUTFILE '/tmp/dbx-probe.txt'", DatabaseType::Mysql));
|
||||
assert!(!is_dangerous_sql_for_database("SELECT 1", DatabaseType::Postgres));
|
||||
assert!(!is_dangerous_sql_for_database("SELECT 1 INTO unsupported", DatabaseType::Sqlite));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_transaction_statements() {
|
||||
assert_eq!(classify_sql_risk("BEGIN", "postgres").unwrap(), SqlRisk::Transaction);
|
||||
|
||||
+420
-81
@@ -24,6 +24,7 @@ const STORAGE_DB_FILE_NAME: &str = "dbx.db";
|
||||
const APP_STATE_EDITOR_SETTINGS_KEY: &str = "editor_settings";
|
||||
const APP_STATE_OPEN_TABS_KEY: &str = "open_tabs";
|
||||
const APP_STATE_SAVED_SQL_EDITOR_POSITIONS_KEY: &str = "saved_sql_editor_positions";
|
||||
const MCP_GLOBAL_POLICY_KEY: &str = "mcp_global_policy";
|
||||
const USER_DATA_TABLES: &[&str] = &[
|
||||
"connections",
|
||||
"connection_secrets",
|
||||
@@ -165,6 +166,34 @@ pub struct DesktopSettings {
|
||||
pub sidebar_table_page_size: usize,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct McpGlobalPolicy {
|
||||
pub read_only: bool,
|
||||
#[serde(default)]
|
||||
pub allow_dangerous_sql: bool,
|
||||
pub allowed_connection_ids: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct McpGlobalPolicyState {
|
||||
pub configured: bool,
|
||||
pub read_only: bool,
|
||||
pub allow_dangerous_sql: bool,
|
||||
pub allowed_connection_ids: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
impl McpGlobalPolicyState {
|
||||
pub fn policy(&self) -> McpGlobalPolicy {
|
||||
McpGlobalPolicy {
|
||||
read_only: self.read_only,
|
||||
allow_dangerous_sql: self.allow_dangerous_sql,
|
||||
allowed_connection_ids: self.allowed_connection_ids.clone(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn default_sidebar_table_page_size() -> usize {
|
||||
1000
|
||||
}
|
||||
@@ -1088,7 +1117,7 @@ impl Storage {
|
||||
};
|
||||
match serde_json::from_str::<serde_json::Value>(&json).map_err(|e| e.to_string())? {
|
||||
serde_json::Value::Object(map) => Ok(map),
|
||||
_ => Ok(serde_json::Map::new()),
|
||||
_ => Err("app settings JSON must be an object".to_string()),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1096,8 +1125,23 @@ impl Storage {
|
||||
&self,
|
||||
settings: &serde_json::Map<String, serde_json::Value>,
|
||||
) -> Result<(), String> {
|
||||
let json = serde_json::Value::Object(settings.clone()).to_string();
|
||||
let mut settings = settings.clone();
|
||||
self.with_conn(move |conn| {
|
||||
// The dedicated policy writer is the only owner of this key. Keep
|
||||
// its latest value across overlapping legacy settings saves.
|
||||
let current: Option<String> = conn
|
||||
.query_row("SELECT settings_json FROM app_settings WHERE id = 1", [], |row| row.get(0))
|
||||
.optional()
|
||||
.map_err(|e| e.to_string())?;
|
||||
settings.remove(MCP_GLOBAL_POLICY_KEY);
|
||||
if let Some(current) = current {
|
||||
let current = serde_json::from_str::<serde_json::Map<String, serde_json::Value>>(¤t)
|
||||
.map_err(|e| format!("invalid app settings JSON: {e}"))?;
|
||||
if let Some(policy) = current.get(MCP_GLOBAL_POLICY_KEY) {
|
||||
settings.insert(MCP_GLOBAL_POLICY_KEY.to_string(), policy.clone());
|
||||
}
|
||||
}
|
||||
let json = serde_json::Value::Object(settings).to_string();
|
||||
conn.execute("INSERT OR REPLACE INTO app_settings (id, settings_json) VALUES (1, ?1)", [json])
|
||||
.map(|_| ())
|
||||
.map_err(|e| e.to_string())
|
||||
@@ -1116,6 +1160,68 @@ impl Storage {
|
||||
Ok(settings.get("password_hash").and_then(|v| v.as_str()).map(|s| s.to_string()))
|
||||
}
|
||||
|
||||
pub async fn load_mcp_global_policy(&self) -> Result<McpGlobalPolicyState, String> {
|
||||
let result = self
|
||||
.with_conn(|conn| {
|
||||
let json: Option<String> = conn
|
||||
.query_row("SELECT settings_json FROM app_settings WHERE id = 1", [], |row| row.get(0))
|
||||
.optional()
|
||||
.map_err(|e| e.to_string())?;
|
||||
let Some(json) = json else {
|
||||
let policy = McpGlobalPolicy::default();
|
||||
return Ok(McpGlobalPolicyState {
|
||||
configured: false,
|
||||
read_only: policy.read_only,
|
||||
allow_dangerous_sql: policy.allow_dangerous_sql,
|
||||
allowed_connection_ids: policy.allowed_connection_ids,
|
||||
});
|
||||
};
|
||||
let settings = serde_json::from_str::<serde_json::Map<String, serde_json::Value>>(&json)
|
||||
.map_err(|e| format!("invalid app settings JSON: {e}"))?;
|
||||
let Some(value) = settings.get(MCP_GLOBAL_POLICY_KEY) else {
|
||||
let policy = McpGlobalPolicy::default();
|
||||
return Ok(McpGlobalPolicyState {
|
||||
configured: false,
|
||||
read_only: policy.read_only,
|
||||
allow_dangerous_sql: policy.allow_dangerous_sql,
|
||||
allowed_connection_ids: policy.allowed_connection_ids,
|
||||
});
|
||||
};
|
||||
let policy = serde_json::from_value::<McpGlobalPolicy>(value.clone())
|
||||
.map_err(|e| format!("invalid MCP policy: {e}"))?;
|
||||
Ok(McpGlobalPolicyState {
|
||||
configured: true,
|
||||
read_only: policy.read_only,
|
||||
allow_dangerous_sql: policy.allow_dangerous_sql,
|
||||
allowed_connection_ids: policy.allowed_connection_ids,
|
||||
})
|
||||
})
|
||||
.await;
|
||||
result.map_err(|e| format!("MCP_POLICY_UNAVAILABLE: {e}"))
|
||||
}
|
||||
|
||||
pub async fn save_mcp_global_policy(&self, policy: &McpGlobalPolicy) -> Result<(), String> {
|
||||
let policy = serde_json::to_value(policy).map_err(|e| format!("MCP_POLICY_UNAVAILABLE: {e}"))?;
|
||||
self.with_conn(move |conn| {
|
||||
let current: Option<String> = conn
|
||||
.query_row("SELECT settings_json FROM app_settings WHERE id = 1", [], |row| row.get(0))
|
||||
.optional()
|
||||
.map_err(|e| e.to_string())?;
|
||||
let mut settings = match current {
|
||||
Some(json) => serde_json::from_str::<serde_json::Map<String, serde_json::Value>>(&json)
|
||||
.map_err(|e| format!("invalid app settings JSON: {e}"))?,
|
||||
None => serde_json::Map::new(),
|
||||
};
|
||||
settings.insert(MCP_GLOBAL_POLICY_KEY.to_string(), policy);
|
||||
let json = serde_json::to_string(&settings).map_err(|e| e.to_string())?;
|
||||
conn.execute("INSERT OR REPLACE INTO app_settings (id, settings_json) VALUES (1, ?1)", [json])
|
||||
.map(|_| ())
|
||||
.map_err(|e| e.to_string())
|
||||
})
|
||||
.await
|
||||
.map_err(|e| format!("MCP_POLICY_UNAVAILABLE: {e}"))
|
||||
}
|
||||
|
||||
pub async fn save_desktop_settings(&self, desktop_settings: &DesktopSettings) -> Result<(), String> {
|
||||
let mut settings = self.load_app_settings_json().await?;
|
||||
settings.remove("run_in_background");
|
||||
@@ -1456,6 +1562,121 @@ impl Storage {
|
||||
|
||||
// Connections
|
||||
|
||||
fn load_mcp_global_policy_in_tx(tx: &rusqlite::Transaction<'_>) -> Result<McpGlobalPolicy, String> {
|
||||
let settings_json: Option<String> = tx
|
||||
.query_row("SELECT settings_json FROM app_settings WHERE id = 1", [], |row| row.get(0))
|
||||
.optional()
|
||||
.map_err(|e| format!("MCP_POLICY_UNAVAILABLE: {e}"))?;
|
||||
Ok(match settings_json {
|
||||
Some(json) => {
|
||||
let settings = serde_json::from_str::<serde_json::Map<String, serde_json::Value>>(&json)
|
||||
.map_err(|e| format!("MCP_POLICY_UNAVAILABLE: invalid app settings JSON: {e}"))?;
|
||||
match settings.get(MCP_GLOBAL_POLICY_KEY) {
|
||||
Some(value) => serde_json::from_value::<McpGlobalPolicy>(value.clone())
|
||||
.map_err(|e| format!("MCP_POLICY_UNAVAILABLE: invalid MCP policy: {e}"))?,
|
||||
None => McpGlobalPolicy::default(),
|
||||
}
|
||||
}
|
||||
None => McpGlobalPolicy::default(),
|
||||
})
|
||||
}
|
||||
|
||||
fn ensure_mcp_connection_change_allowed_in_tx(
|
||||
tx: &rusqlite::Transaction<'_>,
|
||||
target_connection_id: Option<&str>,
|
||||
) -> Result<(), String> {
|
||||
let policy = load_mcp_global_policy_in_tx(tx)?;
|
||||
if policy.read_only {
|
||||
return Err(
|
||||
"MCP_READ_ONLY: DBX global MCP read-only mode is enabled. Connection changes are blocked.".to_string()
|
||||
);
|
||||
}
|
||||
if let Some(connection_id) = target_connection_id {
|
||||
if policy.allowed_connection_ids.as_ref().is_some_and(|ids| !ids.iter().any(|id| id == connection_id)) {
|
||||
return Err(format!(
|
||||
"CONNECTION_OUT_OF_SCOPE: connection '{connection_id}' is not allowed by the current DBX MCP policy"
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn persist_connection_in_tx(tx: &rusqlite::Transaction<'_>, config: &ConnectionConfig) -> Result<(), String> {
|
||||
let config = config.clone().canonicalized();
|
||||
let config_id = config.id.clone();
|
||||
let mut sanitized = config.clone();
|
||||
sanitized.password = String::new();
|
||||
scrub_transport_layer_secrets(&mut sanitized);
|
||||
sanitized.redis_sentinel_password = String::new();
|
||||
sanitized.connection_string = None;
|
||||
sanitized.init_script = None;
|
||||
scrub_mq_auth_secrets(&mut sanitized);
|
||||
scrub_mq_token_signing_secret(&mut sanitized);
|
||||
scrub_nacos_auth_secrets(&mut sanitized);
|
||||
let json = serde_json::to_string(&sanitized).map_err(|e| e.to_string())?;
|
||||
|
||||
tx.execute("INSERT INTO connections (id, config_json) VALUES (?1, ?2)", params![config_id, json])
|
||||
.map_err(|e| e.to_string())?;
|
||||
|
||||
persist_secret_in_tx(tx, &config.id, "password", &config.password)?;
|
||||
delete_secret_prefix_in_tx(tx, &config.id, TRANSPORT_LAYER_SECRET_PREFIX)?;
|
||||
for (index, layer) in config.transport_layers.iter().enumerate() {
|
||||
match layer {
|
||||
TransportLayerConfig::Ssh(ssh) => {
|
||||
persist_secret_in_tx(tx, &config.id, &transport_layer_ssh_password_key(index, layer), &ssh.password)?;
|
||||
persist_secret_in_tx(
|
||||
tx,
|
||||
&config.id,
|
||||
&transport_layer_ssh_key_passphrase_key(index, layer),
|
||||
&ssh.key_passphrase,
|
||||
)?;
|
||||
}
|
||||
TransportLayerConfig::Proxy(proxy) => {
|
||||
persist_secret_in_tx(
|
||||
tx,
|
||||
&config.id,
|
||||
&transport_layer_proxy_password_key(index, layer),
|
||||
&proxy.password,
|
||||
)?;
|
||||
}
|
||||
TransportLayerConfig::HttpTunnel(http) => {
|
||||
persist_secret_in_tx(
|
||||
tx,
|
||||
&config.id,
|
||||
&transport_layer_http_tunnel_token_key(index, layer),
|
||||
&http.token,
|
||||
)?;
|
||||
}
|
||||
}
|
||||
}
|
||||
persist_secret_in_tx(tx, &config.id, "redis_sentinel_password", &config.redis_sentinel_password)?;
|
||||
persist_secret_in_tx(tx, &config.id, "ssh_password", "")?;
|
||||
persist_secret_in_tx(tx, &config.id, "ssh_key_passphrase", "")?;
|
||||
persist_secret_in_tx(tx, &config.id, "proxy_password", "")?;
|
||||
delete_secret_prefix_in_tx(tx, &config.id, SSH_TUNNEL_SECRET_PREFIX)?;
|
||||
if let Some(cs) = &config.connection_string {
|
||||
persist_secret_in_tx(tx, &config.id, "connection_string", cs)?;
|
||||
} else {
|
||||
tx.execute(
|
||||
"DELETE FROM connection_secrets WHERE connection_id = ?1 AND key = ?2",
|
||||
params![config.id, "connection_string"],
|
||||
)
|
||||
.map_err(|e| e.to_string())?;
|
||||
}
|
||||
if let Some(script) = &config.init_script {
|
||||
persist_secret_in_tx(tx, &config.id, "init_script", script)?;
|
||||
} else {
|
||||
tx.execute(
|
||||
"DELETE FROM connection_secrets WHERE connection_id = ?1 AND key = ?2",
|
||||
params![config.id, "init_script"],
|
||||
)
|
||||
.map_err(|e| e.to_string())?;
|
||||
}
|
||||
persist_mq_auth_secrets_in_tx(tx, &config)?;
|
||||
persist_mq_token_signing_secret_in_tx(tx, &config)?;
|
||||
persist_nacos_auth_secrets_in_tx(tx, &config)
|
||||
}
|
||||
|
||||
impl Storage {
|
||||
pub async fn save_connection_metadata_preserving_secrets(
|
||||
&self,
|
||||
@@ -1505,84 +1726,7 @@ impl Storage {
|
||||
tx.execute("DELETE FROM connections", []).map_err(|e| e.to_string())?;
|
||||
|
||||
for config in &configs {
|
||||
let config = config.canonicalized();
|
||||
let config_id = config.id.clone();
|
||||
let mut sanitized = config.clone();
|
||||
sanitized.password = String::new();
|
||||
scrub_transport_layer_secrets(&mut sanitized);
|
||||
sanitized.redis_sentinel_password = String::new();
|
||||
sanitized.connection_string = None;
|
||||
sanitized.init_script = None;
|
||||
scrub_mq_auth_secrets(&mut sanitized);
|
||||
scrub_mq_token_signing_secret(&mut sanitized);
|
||||
scrub_nacos_auth_secrets(&mut sanitized);
|
||||
let json = serde_json::to_string(&sanitized).map_err(|e| e.to_string())?;
|
||||
|
||||
tx.execute("INSERT INTO connections (id, config_json) VALUES (?1, ?2)", params![config_id, json])
|
||||
.map_err(|e| e.to_string())?;
|
||||
|
||||
persist_secret_in_tx(&tx, &config.id, "password", &config.password)?;
|
||||
delete_secret_prefix_in_tx(&tx, &config.id, TRANSPORT_LAYER_SECRET_PREFIX)?;
|
||||
for (index, layer) in config.transport_layers.iter().enumerate() {
|
||||
match layer {
|
||||
TransportLayerConfig::Ssh(ssh) => {
|
||||
persist_secret_in_tx(
|
||||
&tx,
|
||||
&config.id,
|
||||
&transport_layer_ssh_password_key(index, layer),
|
||||
&ssh.password,
|
||||
)?;
|
||||
persist_secret_in_tx(
|
||||
&tx,
|
||||
&config.id,
|
||||
&transport_layer_ssh_key_passphrase_key(index, layer),
|
||||
&ssh.key_passphrase,
|
||||
)?;
|
||||
}
|
||||
TransportLayerConfig::Proxy(proxy) => {
|
||||
persist_secret_in_tx(
|
||||
&tx,
|
||||
&config.id,
|
||||
&transport_layer_proxy_password_key(index, layer),
|
||||
&proxy.password,
|
||||
)?;
|
||||
}
|
||||
TransportLayerConfig::HttpTunnel(http) => {
|
||||
persist_secret_in_tx(
|
||||
&tx,
|
||||
&config.id,
|
||||
&transport_layer_http_tunnel_token_key(index, layer),
|
||||
&http.token,
|
||||
)?;
|
||||
}
|
||||
}
|
||||
}
|
||||
persist_secret_in_tx(&tx, &config.id, "redis_sentinel_password", &config.redis_sentinel_password)?;
|
||||
persist_secret_in_tx(&tx, &config.id, "ssh_password", "")?;
|
||||
persist_secret_in_tx(&tx, &config.id, "ssh_key_passphrase", "")?;
|
||||
persist_secret_in_tx(&tx, &config.id, "proxy_password", "")?;
|
||||
delete_secret_prefix_in_tx(&tx, &config.id, SSH_TUNNEL_SECRET_PREFIX)?;
|
||||
if let Some(cs) = &config.connection_string {
|
||||
persist_secret_in_tx(&tx, &config.id, "connection_string", cs)?;
|
||||
} else {
|
||||
tx.execute(
|
||||
"DELETE FROM connection_secrets WHERE connection_id = ?1 AND key = ?2",
|
||||
params![config.id, "connection_string"],
|
||||
)
|
||||
.map_err(|e| e.to_string())?;
|
||||
}
|
||||
if let Some(script) = &config.init_script {
|
||||
persist_secret_in_tx(&tx, &config.id, "init_script", script)?;
|
||||
} else {
|
||||
tx.execute(
|
||||
"DELETE FROM connection_secrets WHERE connection_id = ?1 AND key = ?2",
|
||||
params![config.id, "init_script"],
|
||||
)
|
||||
.map_err(|e| e.to_string())?;
|
||||
}
|
||||
persist_mq_auth_secrets_in_tx(&tx, &config)?;
|
||||
persist_mq_token_signing_secret_in_tx(&tx, &config)?;
|
||||
persist_nacos_auth_secrets_in_tx(&tx, &config)?;
|
||||
persist_connection_in_tx(&tx, config)?;
|
||||
}
|
||||
|
||||
if configs.is_empty() {
|
||||
@@ -1599,6 +1743,35 @@ impl Storage {
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn add_connection_for_mcp(&self, config: ConnectionConfig) -> Result<ConnectionConfig, String> {
|
||||
let config = config.canonicalized();
|
||||
self.with_conn(move |conn| {
|
||||
let tx = conn.transaction().map_err(|e| e.to_string())?;
|
||||
ensure_mcp_connection_change_allowed_in_tx(&tx, None)?;
|
||||
persist_connection_in_tx(&tx, &config)?;
|
||||
tx.commit().map_err(|e| e.to_string())?;
|
||||
Ok(config)
|
||||
})
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn remove_connection_for_mcp(&self, connection_id: &str) -> Result<bool, String> {
|
||||
let connection_id = connection_id.to_string();
|
||||
self.with_conn(move |conn| {
|
||||
let tx = conn.transaction().map_err(|e| e.to_string())?;
|
||||
ensure_mcp_connection_change_allowed_in_tx(&tx, Some(&connection_id))?;
|
||||
let removed =
|
||||
tx.execute("DELETE FROM connections WHERE id = ?1", [&connection_id]).map_err(|e| e.to_string())? > 0;
|
||||
if removed {
|
||||
tx.execute("DELETE FROM connection_secrets WHERE connection_id = ?1", [&connection_id])
|
||||
.map_err(|e| e.to_string())?;
|
||||
}
|
||||
tx.commit().map_err(|e| e.to_string())?;
|
||||
Ok(removed)
|
||||
})
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn save_connection_database_info(
|
||||
&self,
|
||||
connection_id: &str,
|
||||
@@ -2864,7 +3037,10 @@ fn map_from_sql_err(err: serde_json::Error) -> rusqlite::Error {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{maybe_import_user_data_db, DataDbImportResult, DesktopIconTheme, DesktopSettings, Storage};
|
||||
use super::{
|
||||
maybe_import_user_data_db, DataDbImportResult, DesktopIconTheme, DesktopSettings, McpGlobalPolicy,
|
||||
McpGlobalPolicyState, Storage, MCP_GLOBAL_POLICY_KEY,
|
||||
};
|
||||
use crate::connection_secrets::{
|
||||
MQ_AUTH_PASSWORD_KEY, MQ_AUTH_TOKEN_KEY, MQ_TOKEN_SIGNING_KEY, NACOS_AUTH_PASSWORD_KEY,
|
||||
};
|
||||
@@ -3382,6 +3558,169 @@ mod tests {
|
||||
assert_eq!(storage.load_desktop_settings().await.unwrap(), DesktopSettings::default());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_global_policy_defaults_unconfigured_and_roundtrips_atomically() {
|
||||
let path = temp_db_path("mcp-global-policy");
|
||||
let storage = Storage::open(&path).await.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
storage.load_mcp_global_policy().await.unwrap(),
|
||||
McpGlobalPolicyState {
|
||||
configured: false,
|
||||
read_only: false,
|
||||
allow_dangerous_sql: false,
|
||||
allowed_connection_ids: None,
|
||||
}
|
||||
);
|
||||
|
||||
storage.save_password_hash("preserved").await.unwrap();
|
||||
storage
|
||||
.save_mcp_global_policy(&McpGlobalPolicy {
|
||||
read_only: true,
|
||||
allow_dangerous_sql: true,
|
||||
allowed_connection_ids: Some(vec!["conn-1".to_string(), "conn-2".to_string()]),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
storage.load_mcp_global_policy().await.unwrap(),
|
||||
McpGlobalPolicyState {
|
||||
configured: true,
|
||||
read_only: true,
|
||||
allow_dangerous_sql: true,
|
||||
allowed_connection_ids: Some(vec!["conn-1".to_string(), "conn-2".to_string()]),
|
||||
}
|
||||
);
|
||||
assert_eq!(storage.load_password_hash().await.unwrap().as_deref(), Some("preserved"));
|
||||
let settings = storage.load_app_settings_json().await.unwrap();
|
||||
assert_eq!(settings[MCP_GLOBAL_POLICY_KEY]["readOnly"], true);
|
||||
assert_eq!(settings[MCP_GLOBAL_POLICY_KEY]["allowDangerousSql"], true);
|
||||
assert_eq!(settings[MCP_GLOBAL_POLICY_KEY]["allowedConnectionIds"][0], "conn-1");
|
||||
assert!(settings[MCP_GLOBAL_POLICY_KEY].get("configured").is_none());
|
||||
|
||||
storage.save_desktop_settings(&DesktopSettings::default()).await.unwrap();
|
||||
assert!(storage.load_mcp_global_policy().await.unwrap().read_only);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_global_policy_fails_closed_on_malformed_settings() {
|
||||
let path = temp_db_path("mcp-global-policy-malformed");
|
||||
let storage = Storage::open(&path).await.unwrap();
|
||||
storage
|
||||
.with_conn(|conn| {
|
||||
conn.execute(
|
||||
"INSERT OR REPLACE INTO app_settings (id, settings_json) VALUES (1, ?1)",
|
||||
[r#"{"mcp_global_policy":{"readOnly":"yes","allowedConnectionIds":null}}"#],
|
||||
)
|
||||
.map(|_| ())
|
||||
.map_err(|e| e.to_string())
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let error = storage.load_mcp_global_policy().await.unwrap_err();
|
||||
assert!(error.starts_with("MCP_POLICY_UNAVAILABLE:"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn malformed_app_settings_cannot_be_silently_replaced_by_an_unrelated_save() {
|
||||
let path = temp_db_path("mcp-global-policy-invalid-settings-shape");
|
||||
let storage = Storage::open(&path).await.unwrap();
|
||||
storage
|
||||
.with_conn(|conn| {
|
||||
conn.execute("INSERT OR REPLACE INTO app_settings (id, settings_json) VALUES (1, ?1)", ["[]"])
|
||||
.map(|_| ())
|
||||
.map_err(|e| e.to_string())
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(storage.load_mcp_global_policy().await.unwrap_err().starts_with("MCP_POLICY_UNAVAILABLE:"));
|
||||
assert!(storage.save_password_hash("must-not-reset-policy").await.is_err());
|
||||
let raw = storage
|
||||
.with_conn(|conn| {
|
||||
conn.query_row("SELECT settings_json FROM app_settings WHERE id = 1", [], |row| row.get::<_, String>(0))
|
||||
.map_err(|e| e.to_string())
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(raw, "[]");
|
||||
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_global_policy_defaults_dangerous_sql_to_disabled_for_existing_settings() {
|
||||
let path = temp_db_path("mcp-global-policy-existing");
|
||||
let storage = Storage::open(&path).await.unwrap();
|
||||
storage
|
||||
.with_conn(|conn| {
|
||||
conn.execute(
|
||||
"INSERT OR REPLACE INTO app_settings (id, settings_json) VALUES (1, ?1)",
|
||||
[r#"{"mcp_global_policy":{"readOnly":false,"allowedConnectionIds":null}}"#],
|
||||
)
|
||||
.map(|_| ())
|
||||
.map_err(|e| e.to_string())
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let policy = storage.load_mcp_global_policy().await.unwrap();
|
||||
assert!(policy.configured);
|
||||
assert!(!policy.allow_dangerous_sql);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_connection_mutations_are_atomic_and_recheck_policy() {
|
||||
let path = temp_db_path("mcp-connection-mutation-guard");
|
||||
let storage = Storage::open(&path).await.unwrap();
|
||||
let kept = mq_connection("kept", "kept-token");
|
||||
let removed = mq_connection("removed", "removed-token");
|
||||
storage.save_connections(&[kept.clone(), removed.clone()]).await.unwrap();
|
||||
|
||||
storage
|
||||
.save_mcp_global_policy(&McpGlobalPolicy {
|
||||
read_only: false,
|
||||
allow_dangerous_sql: false,
|
||||
allowed_connection_ids: Some(vec![kept.id.clone()]),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
let error = storage.remove_connection_for_mcp(&removed.id).await.unwrap_err();
|
||||
assert!(error.starts_with("CONNECTION_OUT_OF_SCOPE:"));
|
||||
|
||||
let mut concurrently_updated = removed.clone();
|
||||
concurrently_updated.host = "updated-by-web-ui".to_string();
|
||||
storage.save_connections(&[kept.clone(), concurrently_updated.clone()]).await.unwrap();
|
||||
let added = mq_connection("added", "added-token");
|
||||
storage.add_connection_for_mcp(added.clone()).await.unwrap();
|
||||
let after_add = storage.load_connections().await.unwrap();
|
||||
assert_eq!(after_add.len(), 3);
|
||||
assert_eq!(
|
||||
after_add.iter().find(|config| config.id == concurrently_updated.id).map(|config| config.host.as_str()),
|
||||
Some("updated-by-web-ui")
|
||||
);
|
||||
|
||||
storage
|
||||
.save_mcp_global_policy(&McpGlobalPolicy {
|
||||
read_only: true,
|
||||
allow_dangerous_sql: false,
|
||||
allowed_connection_ids: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
let error = storage.remove_connection_for_mcp(&kept.id).await.unwrap_err();
|
||||
assert!(error.starts_with("MCP_READ_ONLY:"));
|
||||
assert_eq!(storage.load_connections().await.unwrap().len(), 3);
|
||||
|
||||
// Non-MCP callers remain governed by the ordinary DBX UI permissions.
|
||||
storage.save_connections(std::slice::from_ref(&kept)).await.unwrap();
|
||||
assert_eq!(storage.load_connections().await.unwrap()[0].id, kept.id);
|
||||
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn desktop_settings_fall_back_to_legacy_background_preference() {
|
||||
let path = temp_db_path("desktop-settings-legacy-background");
|
||||
|
||||
Reference in new issue
Block a user