mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
feat(relay): add BUZZ_AGENT_SHARING_DISABLED flag and tighten channel_add_policy default
Security: any relay member could add any agent to any channel because
channel_add_policy defaulted to 'anyone'. Two targeted fixes:
1. Migration 0005 changes the column default from 'anyone' to 'owner_only'
so new agents start safe on all relays.
2. BUZZ_AGENT_SHARING_DISABLED=true enables relay-wide enforcement:
- kind:9000 PUT_USER targeting an agent is rejected unless the actor
is the agent's owner (pre-storage, returns NIP-01 NOTICE).
- kind:10100 setting channel_add_policy='anyone' is blocked (the DB
column is not updated; the event is stored but has no effect).
- On startup, all existing channel_add_policy='anyone' rows are
clamped to 'owner_only' (idempotent, runs every restart while
the flag is set).
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
co-authored by
Will Pfleger
parent
4481f8fd5a
commit
9dc0dd48db
@@ -625,6 +625,15 @@ impl Db {
|
||||
user::set_channel_add_policy(&self.pool, pubkey, policy).await
|
||||
}
|
||||
|
||||
/// Clamp all `channel_add_policy = 'anyone'` rows to `'owner_only'`.
|
||||
///
|
||||
/// Used on startup when `BUZZ_AGENT_SHARING_DISABLED` is set to retroactively
|
||||
/// enforce the restriction on agents that were configured before the flag existed.
|
||||
/// Returns the number of rows updated.
|
||||
pub async fn clamp_anyone_channel_add_policy(&self) -> Result<u64> {
|
||||
user::clamp_anyone_channel_add_policy(&self.pool).await
|
||||
}
|
||||
|
||||
/// Find an existing DM by its participant hash.
|
||||
pub async fn find_dm_by_participants(
|
||||
&self,
|
||||
|
||||
@@ -128,7 +128,7 @@ mod tests {
|
||||
fn embedded_migrator_contains_all_schema_migrations() {
|
||||
let migrations: Vec<_> = MIGRATOR.iter().collect();
|
||||
|
||||
assert_eq!(migrations.len(), 3);
|
||||
assert_eq!(migrations.len(), 4);
|
||||
assert_eq!(migrations[0].version, 1);
|
||||
assert_eq!(&*migrations[0].description, "initial schema");
|
||||
assert!(
|
||||
@@ -160,6 +160,19 @@ mod tests {
|
||||
&& migrations[2].sql.as_str().contains("idx_events_not_before"),
|
||||
"third migration should add the NIP-ER reminder columns and index"
|
||||
);
|
||||
|
||||
assert_eq!(migrations[3].version, 5);
|
||||
assert_eq!(
|
||||
&*migrations[3].description,
|
||||
"default channel add policy owner only"
|
||||
);
|
||||
assert!(
|
||||
migrations[3]
|
||||
.sql
|
||||
.as_str()
|
||||
.contains("channel_add_policy SET DEFAULT"),
|
||||
"fifth migration should tighten the channel_add_policy default"
|
||||
);
|
||||
}
|
||||
|
||||
async fn connect_test_pool() -> PgPool {
|
||||
|
||||
@@ -366,6 +366,20 @@ pub async fn set_channel_add_policy(pool: &PgPool, pubkey: &[u8], policy: &str)
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Clamp all `channel_add_policy = 'anyone'` rows to `'owner_only'`.
|
||||
///
|
||||
/// Used on startup when `BUZZ_AGENT_SHARING_DISABLED` is set to retroactively
|
||||
/// enforce the restriction on agents configured before the flag existed.
|
||||
/// Returns the number of rows updated. Safe to call repeatedly (idempotent).
|
||||
pub async fn clamp_anyone_channel_add_policy(pool: &PgPool) -> Result<u64> {
|
||||
let result = sqlx::query(
|
||||
r#"UPDATE users SET channel_add_policy = 'owner_only' WHERE channel_add_policy = 'anyone'"#,
|
||||
)
|
||||
.execute(pool)
|
||||
.await?;
|
||||
Ok(result.rows_affected())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -555,6 +569,66 @@ mod tests {
|
||||
assert!(result.is_err(), "should reject invalid policy value");
|
||||
}
|
||||
|
||||
/// clamp_anyone_channel_add_policy should flip 'anyone' rows to 'owner_only'
|
||||
/// and leave 'owner_only' and 'nobody' rows untouched.
|
||||
#[tokio::test]
|
||||
#[ignore = "requires Postgres"]
|
||||
async fn test_clamp_anyone_channel_add_policy() {
|
||||
let db = setup_db().await;
|
||||
|
||||
let pk_anyone = random_pubkey();
|
||||
let pk_owner_only = random_pubkey();
|
||||
let pk_nobody = random_pubkey();
|
||||
|
||||
ensure_user(&db.pool, &pk_anyone).await.expect("ensure anyone");
|
||||
ensure_user(&db.pool, &pk_owner_only).await.expect("ensure owner_only");
|
||||
ensure_user(&db.pool, &pk_nobody).await.expect("ensure nobody");
|
||||
|
||||
set_channel_add_policy(&db.pool, &pk_anyone, "anyone")
|
||||
.await
|
||||
.expect("set anyone");
|
||||
set_channel_add_policy(&db.pool, &pk_owner_only, "owner_only")
|
||||
.await
|
||||
.expect("set owner_only");
|
||||
set_channel_add_policy(&db.pool, &pk_nobody, "nobody")
|
||||
.await
|
||||
.expect("set nobody");
|
||||
|
||||
let clamped = clamp_anyone_channel_add_policy(&db.pool)
|
||||
.await
|
||||
.expect("clamp");
|
||||
assert!(clamped >= 1, "at least one row should be clamped");
|
||||
|
||||
// 'anyone' → 'owner_only'
|
||||
let (policy, _) = get_agent_channel_policy(&db.pool, &pk_anyone)
|
||||
.await
|
||||
.expect("get policy")
|
||||
.expect("should be Some");
|
||||
assert_eq!(policy, "owner_only", "'anyone' should be clamped to 'owner_only'");
|
||||
|
||||
// 'owner_only' unchanged
|
||||
let (policy, _) = get_agent_channel_policy(&db.pool, &pk_owner_only)
|
||||
.await
|
||||
.expect("get policy")
|
||||
.expect("should be Some");
|
||||
assert_eq!(policy, "owner_only", "'owner_only' should be unchanged");
|
||||
|
||||
// 'nobody' unchanged
|
||||
let (policy, _) = get_agent_channel_policy(&db.pool, &pk_nobody)
|
||||
.await
|
||||
.expect("get policy")
|
||||
.expect("should be Some");
|
||||
assert_eq!(policy, "nobody", "'nobody' should be unchanged");
|
||||
|
||||
// Idempotent: second call returns 0 (no more 'anyone' rows for these pubkeys)
|
||||
let clamped_again = clamp_anyone_channel_add_policy(&db.pool)
|
||||
.await
|
||||
.expect("clamp again");
|
||||
// Can't assert == 0 globally since other tests may have 'anyone' rows,
|
||||
// but the three test rows are already clamped so they won't be counted again.
|
||||
let _ = clamped_again; // just verify it doesn't error
|
||||
}
|
||||
|
||||
// Use the production `escape_like` function directly — no local mirror.
|
||||
use super::escape_like;
|
||||
|
||||
|
||||
@@ -127,6 +127,15 @@ pub struct Config {
|
||||
/// When set, the relay serves the SPA from this directory for browser requests.
|
||||
/// When unset, no static file serving happens (relay behaves as before).
|
||||
pub web_dir: Option<std::path::PathBuf>,
|
||||
|
||||
/// When true, agent sharing is disabled relay-wide:
|
||||
/// - `kind:10100` events setting `channel_add_policy = 'anyone'` are rejected.
|
||||
/// - Third-party `kind:9000` PUT_USER events targeting agents are rejected.
|
||||
/// - On startup, all existing `channel_add_policy = 'anyone'` rows are clamped
|
||||
/// to `'owner_only'`.
|
||||
///
|
||||
/// Default: `false`. Set via `BUZZ_AGENT_SHARING_DISABLED=true`.
|
||||
pub agent_sharing_disabled: bool,
|
||||
}
|
||||
|
||||
impl Config {
|
||||
@@ -194,6 +203,10 @@ impl Config {
|
||||
.map(|v| v == "true" || v == "1")
|
||||
.unwrap_or(false);
|
||||
|
||||
let agent_sharing_disabled = std::env::var("BUZZ_AGENT_SHARING_DISABLED")
|
||||
.map(|v| v == "true" || v == "1")
|
||||
.unwrap_or(false);
|
||||
|
||||
// Note: intentionally not prefixed with BUZZ_ — this is a relay-identity
|
||||
// config that may be shared across multiple services (e.g., ACP agent).
|
||||
let relay_owner_pubkey = std::env::var("RELAY_OWNER_PUBKEY")
|
||||
@@ -400,6 +413,7 @@ impl Config {
|
||||
git_max_concurrent_ops,
|
||||
git_hook_hmac_secret,
|
||||
web_dir,
|
||||
agent_sharing_disabled,
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -440,6 +454,10 @@ mod tests {
|
||||
!config.allow_nip_oa_auth,
|
||||
"allow_nip_oa_auth should default to false"
|
||||
);
|
||||
assert!(
|
||||
!config.agent_sharing_disabled,
|
||||
"agent_sharing_disabled should default to false"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -460,6 +478,24 @@ mod tests {
|
||||
assert_eq!(config.max_frame_bytes, 262_144);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_sharing_disabled_can_be_enabled() {
|
||||
let _guard = ENV_MUTEX.lock().unwrap();
|
||||
std::env::set_var("BUZZ_AGENT_SHARING_DISABLED", "true");
|
||||
let config = Config::from_env().expect("config");
|
||||
std::env::remove_var("BUZZ_AGENT_SHARING_DISABLED");
|
||||
assert!(config.agent_sharing_disabled);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_sharing_disabled_accepts_numeric_one() {
|
||||
let _guard = ENV_MUTEX.lock().unwrap();
|
||||
std::env::set_var("BUZZ_AGENT_SHARING_DISABLED", "1");
|
||||
let config = Config::from_env().expect("config");
|
||||
std::env::remove_var("BUZZ_AGENT_SHARING_DISABLED");
|
||||
assert!(config.agent_sharing_disabled);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn server_domain_auto_derived_from_relay_url() {
|
||||
let _guard = ENV_MUTEX.lock().unwrap();
|
||||
|
||||
@@ -269,6 +269,26 @@ pub async fn validate_admin_event(
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// When agent sharing is disabled relay-wide, only the agent's owner may add it.
|
||||
if state.config.agent_sharing_disabled {
|
||||
if let Some((_policy, owner)) =
|
||||
state.db.get_agent_channel_policy(&target_pubkey).await?
|
||||
{
|
||||
let owner_bytes = owner.ok_or_else(|| {
|
||||
anyhow::anyhow!(
|
||||
"agent sharing disabled — agent has no owner set"
|
||||
)
|
||||
})?;
|
||||
if actor_bytes != owner_bytes {
|
||||
return Err(anyhow::anyhow!(
|
||||
"agent sharing is disabled on this relay — only the agent owner can add this agent"
|
||||
));
|
||||
}
|
||||
// Owner is adding their own agent — allow, skip the per-policy check below.
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
|
||||
// Third-party add: check channel_add_policy on the target.
|
||||
if let Some((policy, owner)) = state.db.get_agent_channel_policy(&target_pubkey).await?
|
||||
{
|
||||
@@ -817,6 +837,12 @@ async fn handle_agent_profile(event: &Event, state: &Arc<AppState>) -> anyhow::R
|
||||
.and_then(|v| v.as_str())
|
||||
.ok_or_else(|| anyhow::anyhow!("kind:10100 missing channel_add_policy field"))?;
|
||||
|
||||
if state.config.agent_sharing_disabled && policy == "anyone" {
|
||||
return Err(anyhow::anyhow!(
|
||||
"agent sharing is disabled on this relay — channel_add_policy 'anyone' is not permitted"
|
||||
));
|
||||
}
|
||||
|
||||
let pubkey_bytes = event.pubkey.to_bytes().to_vec();
|
||||
state.db.ensure_user(&pubkey_bytes).await?;
|
||||
state
|
||||
|
||||
@@ -155,6 +155,19 @@ async fn main() -> anyhow::Result<()> {
|
||||
Err(e) => error!("Failed to backfill d_tags: {e}"),
|
||||
}
|
||||
|
||||
// BUZZ_AGENT_SHARING_DISABLED: retroactively clamp any existing 'anyone'
|
||||
// channel_add_policy rows to 'owner_only'. Idempotent — no-ops when already clamped.
|
||||
if config.agent_sharing_disabled {
|
||||
match db.clamp_anyone_channel_add_policy().await {
|
||||
Ok(0) => info!("BUZZ_AGENT_SHARING_DISABLED: no 'anyone' channel_add_policy rows to clamp"),
|
||||
Ok(n) => tracing::warn!(
|
||||
count = n,
|
||||
"BUZZ_AGENT_SHARING_DISABLED: clamped existing 'anyone' channel_add_policy rows to 'owner_only'"
|
||||
),
|
||||
Err(e) => error!("BUZZ_AGENT_SHARING_DISABLED: failed to clamp channel_add_policy rows: {e}"),
|
||||
}
|
||||
}
|
||||
|
||||
let audit_pool = sqlx::postgres::PgPoolOptions::new()
|
||||
.max_connections(5)
|
||||
.min_connections(1)
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
-- Tighten the default channel_add_policy for new agents from 'anyone' to
|
||||
-- 'owner_only'. Existing rows are unaffected by this migration; the startup
|
||||
-- clamp in main.rs (BUZZ_AGENT_SHARING_DISABLED) handles retroactive clamping
|
||||
-- for relays that need it.
|
||||
ALTER TABLE users
|
||||
ALTER COLUMN channel_add_policy SET DEFAULT 'owner_only';
|
||||
Reference in New Issue
Block a user