From bf5d700a96e80d16bf6119376301fdba2cdbdbff Mon Sep 17 00:00:00 2001 From: kipavy Date: Thu, 10 Sep 2026 20:24:39 +0000 Subject: [PATCH 01/19] feat(perms): add per-member permission override table --- .../042_team_member_permission_overrides.sql | 11 +++++++ src/permissions.rs | 29 ++++++++++++++++++- src/test_support.rs | 16 ++++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 migrations/042_team_member_permission_overrides.sql diff --git a/migrations/042_team_member_permission_overrides.sql b/migrations/042_team_member_permission_overrides.sql new file mode 100644 index 0000000..2e8f6f6 --- /dev/null +++ b/migrations/042_team_member_permission_overrides.sql @@ -0,0 +1,11 @@ +CREATE TABLE team_member_permission_overrides ( + team_id UUID NOT NULL, + user_id UUID NOT NULL, + allow_mask BIGINT NOT NULL DEFAULT 0, + deny_mask BIGINT NOT NULL DEFAULT 0, + updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_by UUID NOT NULL REFERENCES users(id), + PRIMARY KEY (team_id, user_id), + FOREIGN KEY (team_id, user_id) + REFERENCES team_members(team_id, user_id) ON DELETE CASCADE +); diff --git a/src/permissions.rs b/src/permissions.rs index 283db55..9f713a8 100644 --- a/src/permissions.rs +++ b/src/permissions.rs @@ -200,7 +200,7 @@ mod db_tests { //! Requires `TEST_DATABASE_URL`; otherwise each test skips. use super::*; use crate::test_pool_or_skip; - use crate::test_support::{add_member, assign_role, seed_role, seed_team, seed_user}; + use crate::test_support::{add_member, assign_role, seed_role, seed_team, seed_user, set_member_overrides}; #[tokio::test] async fn has_team_permission_reflects_granted_bit() { @@ -321,4 +321,31 @@ mod db_tests { .unwrap() ); } + + #[tokio::test] + async fn overrides_cascade_when_member_is_removed() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let member = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + add_member(&pool, team, member).await; + set_member_overrides(&pool, team, member, PERM_VIEW_SECRETS, 0).await; + + sqlx::query("DELETE FROM team_members WHERE team_id = $1 AND user_id = $2") + .bind(team) + .bind(member) + .execute(&pool) + .await + .unwrap(); + + let remaining: i64 = sqlx::query_scalar( + "SELECT COUNT(*) FROM team_member_permission_overrides WHERE team_id = $1 AND user_id = $2", + ) + .bind(team) + .bind(member) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(remaining, 0); + } } diff --git a/src/test_support.rs b/src/test_support.rs index 5704964..3370bda 100644 --- a/src/test_support.rs +++ b/src/test_support.rs @@ -223,6 +223,22 @@ pub async fn assign_role(pool: &PgPool, team: Uuid, user: Uuid, role: Uuid) { .expect("assign role"); } +/// Set permission overrides for `user` within `team`. +pub async fn set_member_overrides(pool: &PgPool, team: Uuid, user: Uuid, allow: i64, deny: i64) { + sqlx::query( + "INSERT INTO team_member_permission_overrides (team_id, user_id, allow_mask, deny_mask, updated_by) \ + VALUES ($1, $2, $3, $4, $2) \ + ON CONFLICT (team_id, user_id) DO UPDATE SET allow_mask = $3, deny_mask = $4", + ) + .bind(team) + .bind(user) + .bind(allow) + .bind(deny) + .execute(pool) + .await + .expect("set member overrides"); +} + /// Seed a user, grant them a single role with exactly `perms`, add them to /// `team`, and assign the role. Returns the new member's id. The common setup /// for handler authorization tests. From 60f35d1e009f1003934a52dab94275c402edc202 Mon Sep 17 00:00:00 2001 From: kipavy Date: Thu, 10 Sep 2026 20:33:40 +0000 Subject: [PATCH 02/19] refactor(perms): share one effective-permission query between both resolvers --- src/permissions.rs | 66 ++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/src/permissions.rs b/src/permissions.rs index 9f713a8..3c4b07b 100644 --- a/src/permissions.rs +++ b/src/permissions.rs @@ -51,6 +51,12 @@ pub const BUILTIN_ROLES: &[(&str, i64, i32)] = &[ ("connect-only", 28676, 4), // no edit perms today ]; +const PERMISSION_JOINS: &str = r#" + FROM team_members tm + LEFT JOIN team_member_roles tmr ON tmr.team_id = tm.team_id AND tmr.user_id = tm.user_id + LEFT JOIN team_roles tr ON tr.id = tmr.role_id +"#; + /// Union of all role permission bits granted to (team_id, user_id). /// Returns 0 if the user has no roles in the team (or is not a member). async fn effective_permissions( @@ -58,22 +64,20 @@ async fn effective_permissions( team_id: Uuid, user_id: Uuid, ) -> Result { - sqlx::query_scalar::<_, i64>( - r#" - SELECT COALESCE(bit_or(tr.permissions), 0) - FROM team_member_roles tmr - JOIN team_roles tr ON tr.id = tmr.role_id - WHERE tmr.team_id = $1 AND tmr.user_id = $2 - "#, - ) - .bind(team_id) - .bind(user_id) - .fetch_one(pool) - .await - .map_err(|e| { - error!(error = %e, team_id = %team_id, user_id = %user_id, "Failed to check team permission"); - StatusCode::INTERNAL_SERVER_ERROR - }) + let sql = format!( + "SELECT COALESCE(bit_or(tr.permissions), 0) {PERMISSION_JOINS} \ + WHERE tm.team_id = $1 AND tm.user_id = $2" + ); + sqlx::query_scalar::<_, i64>(&sql) + .bind(team_id) + .bind(user_id) + .fetch_optional(pool) + .await + .map(|v| v.unwrap_or(0)) + .map_err(|e| { + error!(error = %e, team_id = %team_id, user_id = %user_id, "Failed to check team permission"); + StatusCode::INTERNAL_SERVER_ERROR + }) } /// Returns true if any of (team_id, user_id)'s roles grant `permission`. @@ -171,22 +175,20 @@ pub async fn has_any_team_permission( if team_ids.is_empty() { return Ok(false); } - let effective = sqlx::query_scalar::<_, i64>( - r#" - SELECT COALESCE(bit_or(tr.permissions), 0) - FROM team_member_roles tmr - JOIN team_roles tr ON tr.id = tmr.role_id - WHERE tmr.team_id = ANY($1) AND tmr.user_id = $2 - "#, - ) - .bind(team_ids) - .bind(user_id) - .fetch_one(pool) - .await - .map_err(|e| { - error!(error = %e, user_id = %user_id, "Failed to check any-team permission"); - StatusCode::INTERNAL_SERVER_ERROR - })?; + let sql = format!( + "SELECT COALESCE(bit_or(tr.permissions), 0) {PERMISSION_JOINS} \ + WHERE tm.team_id = ANY($1) AND tm.user_id = $2" + ); + let effective = sqlx::query_scalar::<_, i64>(&sql) + .bind(team_ids) + .bind(user_id) + .fetch_optional(pool) + .await + .map(|v| v.unwrap_or(0)) + .map_err(|e| { + error!(error = %e, user_id = %user_id, "Failed to check any-team permission"); + StatusCode::INTERNAL_SERVER_ERROR + })?; Ok((effective & permission) != 0) } From efa9c1ab9690f504316731e6ea76d495b62d0b5e Mon Sep 17 00:00:00 2001 From: kipavy Date: Thu, 10 Sep 2026 20:43:51 +0000 Subject: [PATCH 03/19] feat(perms): apply per-member allow/deny overlay in the team resolver --- src/permissions.rs | 81 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 3 deletions(-) diff --git a/src/permissions.rs b/src/permissions.rs index 3c4b07b..82ebf60 100644 --- a/src/permissions.rs +++ b/src/permissions.rs @@ -55,17 +55,22 @@ const PERMISSION_JOINS: &str = r#" FROM team_members tm LEFT JOIN team_member_roles tmr ON tmr.team_id = tm.team_id AND tmr.user_id = tm.user_id LEFT JOIN team_roles tr ON tr.id = tmr.role_id + LEFT JOIN team_member_permission_overrides o + ON o.team_id = tm.team_id AND o.user_id = tm.user_id "#; -/// Union of all role permission bits granted to (team_id, user_id). -/// Returns 0 if the user has no roles in the team (or is not a member). +// MAX pulls the single override row (join is one-to-at-most-one) into the aggregate. +const EFFECTIVE_EXPR: &str = "(COALESCE(bit_or(tr.permissions), 0) | COALESCE(MAX(o.allow_mask), 0)) \ + & ~COALESCE(MAX(o.deny_mask), 0)"; + +/// `(roleUnion | allow) & ~deny`. Returns 0 if the user is not a member. async fn effective_permissions( pool: &PgPool, team_id: Uuid, user_id: Uuid, ) -> Result { let sql = format!( - "SELECT COALESCE(bit_or(tr.permissions), 0) {PERMISSION_JOINS} \ + "SELECT {EFFECTIVE_EXPR} {PERMISSION_JOINS} \ WHERE tm.team_id = $1 AND tm.user_id = $2" ); sqlx::query_scalar::<_, i64>(&sql) @@ -350,4 +355,74 @@ mod db_tests { .unwrap(); assert_eq!(remaining, 0); } + + #[tokio::test] + async fn allow_override_grants_a_bit_no_role_provides() { + let pool = test_pool_or_skip!(); + let user = seed_user(&pool).await; + let team = seed_team(&pool, user).await; + let role = seed_role(&pool, team, "r", PERM_CONNECT).await; + add_member(&pool, team, user).await; + assign_role(&pool, team, user, role).await; + crate::test_support::set_member_overrides(&pool, team, user, PERM_VIEW_SECRETS, 0).await; + + assert!(has_team_permission(&pool, team, user, PERM_VIEW_SECRETS).await.unwrap()); + assert!(has_team_permission(&pool, team, user, PERM_CONNECT).await.unwrap()); + } + + #[tokio::test] + async fn allow_override_works_for_a_member_with_no_roles() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let member = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + add_member(&pool, team, member).await; + crate::test_support::set_member_overrides(&pool, team, member, PERM_CONNECT, 0).await; + + assert!(has_team_permission(&pool, team, member, PERM_CONNECT).await.unwrap()); + } + + #[tokio::test] + async fn deny_override_beats_a_granting_role() { + let pool = test_pool_or_skip!(); + let user = seed_user(&pool).await; + let team = seed_team(&pool, user).await; + let role = seed_role(&pool, team, "r", PERM_VIEW_SECRETS | PERM_CONNECT).await; + add_member(&pool, team, user).await; + assign_role(&pool, team, user, role).await; + crate::test_support::set_member_overrides(&pool, team, user, 0, PERM_VIEW_SECRETS).await; + + assert!(!has_team_permission(&pool, team, user, PERM_VIEW_SECRETS).await.unwrap()); + assert!(has_team_permission(&pool, team, user, PERM_CONNECT).await.unwrap()); + } + + #[tokio::test] + async fn deny_survives_a_newly_assigned_role_granting_the_same_bit() { + let pool = test_pool_or_skip!(); + let user = seed_user(&pool).await; + let team = seed_team(&pool, user).await; + let role_a = seed_role(&pool, team, "a", PERM_CONNECT).await; + add_member(&pool, team, user).await; + assign_role(&pool, team, user, role_a).await; + crate::test_support::set_member_overrides(&pool, team, user, 0, PERM_VIEW_SECRETS).await; + + let role_b = seed_role(&pool, team, "b", PERM_VIEW_SECRETS).await; + assign_role(&pool, team, user, role_b).await; + + assert!(!has_team_permission(&pool, team, user, PERM_VIEW_SECRETS).await.unwrap()); + } + + #[tokio::test] + async fn deny_wins_when_a_bit_is_in_both_masks() { + let pool = test_pool_or_skip!(); + let user = seed_user(&pool).await; + let team = seed_team(&pool, user).await; + add_member(&pool, team, user).await; + crate::test_support::set_member_overrides( + &pool, team, user, PERM_VIEW_SECRETS, PERM_VIEW_SECRETS, + ) + .await; + + assert!(!has_team_permission(&pool, team, user, PERM_VIEW_SECRETS).await.unwrap()); + } } From d9d9069739595e94382c5f87e50cda2f59420cb6 Mon Sep 17 00:00:00 2001 From: kipavy Date: Thu, 10 Sep 2026 20:58:46 +0000 Subject: [PATCH 04/19] fix(perms): resolve multi-team permission checks per team, not across the union --- src/permissions.rs | 79 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 73 insertions(+), 6 deletions(-) diff --git a/src/permissions.rs b/src/permissions.rs index 82ebf60..2da3d6d 100644 --- a/src/permissions.rs +++ b/src/permissions.rs @@ -181,21 +181,25 @@ pub async fn has_any_team_permission( return Ok(false); } let sql = format!( - "SELECT COALESCE(bit_or(tr.permissions), 0) {PERMISSION_JOINS} \ - WHERE tm.team_id = ANY($1) AND tm.user_id = $2" + "SELECT COALESCE(bool_or(granted), false) FROM ( \ + SELECT (({EFFECTIVE_EXPR}) & $3) <> 0 AS granted \ + {PERMISSION_JOINS} \ + WHERE tm.team_id = ANY($1) AND tm.user_id = $2 \ + GROUP BY tm.team_id \ + ) per_team" ); - let effective = sqlx::query_scalar::<_, i64>(&sql) + let granted = sqlx::query_scalar::<_, bool>(&sql) .bind(team_ids) .bind(user_id) - .fetch_optional(pool) + .bind(permission) + .fetch_one(pool) .await - .map(|v| v.unwrap_or(0)) .map_err(|e| { error!(error = %e, user_id = %user_id, "Failed to check any-team permission"); StatusCode::INTERNAL_SERVER_ERROR })?; - Ok((effective & permission) != 0) + Ok(granted) } #[cfg(test)] @@ -425,4 +429,67 @@ mod db_tests { assert!(!has_team_permission(&pool, team, user, PERM_VIEW_SECRETS).await.unwrap()); } + + #[tokio::test] + async fn deny_in_one_team_does_not_suppress_a_grant_in_another() { + let pool = test_pool_or_skip!(); + let user = seed_user(&pool).await; + let team_a = seed_team(&pool, user).await; + let team_b = seed_team(&pool, user).await; + + let role_a = seed_role(&pool, team_a, "a", PERM_VIEW_AUDIT_LOG).await; + add_member(&pool, team_a, user).await; + assign_role(&pool, team_a, user, role_a).await; + crate::test_support::set_member_overrides(&pool, team_a, user, 0, PERM_VIEW_AUDIT_LOG).await; + + let role_b = seed_role(&pool, team_b, "b", PERM_VIEW_AUDIT_LOG).await; + add_member(&pool, team_b, user).await; + assign_role(&pool, team_b, user, role_b).await; + + assert!( + has_any_team_permission(&pool, &[team_a, team_b], user, PERM_VIEW_AUDIT_LOG) + .await + .unwrap(), + "team_b still grants the bit; team_a's deny must not reach across teams" + ); + assert!( + !has_team_permission(&pool, team_a, user, PERM_VIEW_AUDIT_LOG).await.unwrap(), + "team_a's own deny still applies inside team_a" + ); + } + + #[tokio::test] + async fn deny_override_applies_to_multi_team_checks() { + let pool = test_pool_or_skip!(); + let user = seed_user(&pool).await; + let team = seed_team(&pool, user).await; + let role = seed_role(&pool, team, "r", PERM_VIEW_AUDIT_LOG).await; + add_member(&pool, team, user).await; + assign_role(&pool, team, user, role).await; + crate::test_support::set_member_overrides(&pool, team, user, 0, PERM_VIEW_AUDIT_LOG).await; + + assert!( + !has_any_team_permission(&pool, &[team], user, PERM_VIEW_AUDIT_LOG) + .await + .unwrap(), + "a deny override must be honoured on the multi-team path, not only the single-team one" + ); + } + + #[tokio::test] + async fn allow_override_applies_to_multi_team_checks() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let member = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + add_member(&pool, team, member).await; + crate::test_support::set_member_overrides(&pool, team, member, PERM_VIEW_AUDIT_LOG, 0).await; + + assert!( + has_any_team_permission(&pool, &[team], member, PERM_VIEW_AUDIT_LOG) + .await + .unwrap(), + "an allow override must grant on the multi-team path" + ); + } } From 264ac3a9db972027e8dac48f04ec540f1ce0d4f7 Mon Sep 17 00:00:00 2001 From: kipavy Date: Thu, 10 Sep 2026 21:11:18 +0000 Subject: [PATCH 05/19] feat(teams): serve per-member permission masks on the team and member endpoints --- src/models/team.rs | 2 ++ src/routes/teams.rs | 64 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/models/team.rs b/src/models/team.rs index 75c2ac4..c853e07 100644 --- a/src/models/team.rs +++ b/src/models/team.rs @@ -36,4 +36,6 @@ pub struct TeamMember { pub invited_by_display_name: Option, pub joined_at: DateTime, pub role_ids: Vec, + pub permission_allow: i64, + pub permission_deny: i64, } diff --git a/src/routes/teams.rs b/src/routes/teams.rs index 1741b07..ae6878a 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -200,6 +200,8 @@ pub struct TeamWithRole { pub owner_tier: String, pub created_at: chrono::DateTime, pub role_ids: Vec, + pub permission_allow: i64, + pub permission_deny: i64, } pub async fn list_teams( @@ -207,13 +209,15 @@ pub async fn list_teams( axum::Extension(auth): axum::Extension, ) -> Result>, StatusCode> { // Returns one row per (team, role) — aggregated in Rust - let rows = sqlx::query_as::<_, (Uuid, String, Uuid, String, chrono::DateTime, Option)>( + let rows = sqlx::query_as::<_, (Uuid, String, Uuid, String, chrono::DateTime, Option, i64, i64)>( r#" - SELECT t.id, t.name, t.owner_id, u.subscription_tier, t.created_at, tmr.role_id + SELECT t.id, t.name, t.owner_id, u.subscription_tier, t.created_at, tmr.role_id, + COALESCE(o.allow_mask, 0), COALESCE(o.deny_mask, 0) FROM teams t JOIN team_members tm ON tm.team_id = t.id AND tm.user_id = $1 JOIN users u ON u.id = t.owner_id LEFT JOIN team_member_roles tmr ON tmr.team_id = t.id AND tmr.user_id = $1 + LEFT JOIN team_member_permission_overrides o ON o.team_id = t.id AND o.user_id = $1 ORDER BY t.created_at ASC, tmr.role_id ASC NULLS LAST "#, ) @@ -226,7 +230,7 @@ pub async fn list_teams( })?; let mut teams: Vec = Vec::new(); - for (id, name, owner_id, owner_tier, created_at, role_id) in rows { + for (id, name, owner_id, owner_tier, created_at, role_id, permission_allow, permission_deny) in rows { match teams.last_mut() { Some(last) if last.id == id => { if let Some(rid) = role_id { @@ -241,6 +245,8 @@ pub async fn list_teams( owner_tier, created_at, role_ids: role_id.into_iter().collect(), + permission_allow, + permission_deny, }); } } @@ -303,23 +309,20 @@ pub async fn list_members( let rows = sqlx::query_as::< _, ( - Uuid, - Uuid, - Option, - chrono::DateTime, - String, - String, - Option, - Option, + Uuid, Uuid, Option, chrono::DateTime, + String, String, Option, Option, i64, i64, ), >( r#" SELECT tm.team_id, tm.user_id, inv.handle AS invited_by_display_name, tm.joined_at, - u.handle AS display_name, u.handle, u.public_key, tmr.role_id + u.handle AS display_name, u.handle, u.public_key, tmr.role_id, + COALESCE(o.allow_mask, 0), COALESCE(o.deny_mask, 0) FROM team_members tm JOIN users u ON u.id = tm.user_id LEFT JOIN users inv ON inv.id = tm.invited_by LEFT JOIN team_member_roles tmr ON tmr.team_id = tm.team_id AND tmr.user_id = tm.user_id + LEFT JOIN team_member_permission_overrides o + ON o.team_id = tm.team_id AND o.user_id = tm.user_id WHERE tm.team_id = $1 ORDER BY tm.joined_at ASC, tmr.role_id ASC NULLS LAST "#, @@ -333,7 +336,8 @@ pub async fn list_members( })?; let mut members: Vec = Vec::new(); - for (t_id, user_id, invited_by_display_name, joined_at, display_name, handle, public_key, role_id) in rows + for (t_id, user_id, invited_by_display_name, joined_at, display_name, handle, public_key, + role_id, permission_allow, permission_deny) in rows { match members.last_mut() { Some(last) if last.member.user_id == user_id => { @@ -353,6 +357,8 @@ pub async fn list_members( invited_by_display_name, joined_at, role_ids: role_id.into_iter().collect(), + permission_allow, + permission_deny, }, }); } @@ -2493,3 +2499,35 @@ mod search_tests { assert!(hits.iter().any(|r| r.user_id == them)); } } + +#[cfg(test)] +mod override_response_tests { + use crate::permissions::{PERM_CONNECT, PERM_VIEW_SECRETS}; + use crate::test_pool_or_skip; + use crate::test_support::{add_member, seed_team, seed_user, set_member_overrides}; + + #[tokio::test] + async fn member_masks_are_read_back_from_the_database() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let member = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + add_member(&pool, team, member).await; + set_member_overrides(&pool, team, member, PERM_CONNECT, PERM_VIEW_SECRETS).await; + + let rows = sqlx::query_as::<_, (i64, i64)>( + "SELECT COALESCE(o.allow_mask, 0), COALESCE(o.deny_mask, 0) \ + FROM team_members tm \ + LEFT JOIN team_member_permission_overrides o \ + ON o.team_id = tm.team_id AND o.user_id = tm.user_id \ + WHERE tm.team_id = $1 AND tm.user_id = $2", + ) + .bind(team) + .bind(member) + .fetch_one(&pool) + .await + .unwrap(); + + assert_eq!(rows, (PERM_CONNECT, PERM_VIEW_SECRETS)); + } +} From c59f5ab9a1136172d133b4cd77bd3e2698848743 Mon Sep 17 00:00:00 2001 From: kipavy Date: Thu, 10 Sep 2026 21:20:46 +0000 Subject: [PATCH 06/19] feat(teams): add endpoint to set a member's permission overrides --- src/main.rs | 4 ++ src/routes/teams.rs | 128 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+) diff --git a/src/main.rs b/src/main.rs index 5d9c33a..dd05db5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -351,6 +351,10 @@ async fn main() { "/v1/teams/:team_id/members/:user_id/roles/:role_id", delete(routes::teams::remove_member_role), ) + .route( + "/v1/teams/:team_id/members/:user_id/permissions", + put(routes::teams::set_member_permissions), + ) .route("/v1/users/search", get(routes::teams::search_users)) .route( "/v1/users/:user_id/public-key", diff --git a/src/routes/teams.rs b/src/routes/teams.rs index ae6878a..80e7aff 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -1206,6 +1206,117 @@ pub async fn remove_member_role( Ok(StatusCode::NO_CONTENT) } +// ─── Set per-member permission overrides ────────────────────────────────────── + +#[derive(Deserialize)] +pub struct SetMemberPermissionsRequest { + pub allow: i64, + pub deny: i64, +} + +fn validate_override_masks(allow: i64, deny: i64) -> Result<(), StatusCode> { + let known = crate::permissions::ALL_PERMISSIONS; + if allow < 0 || deny < 0 || (allow & !known) != 0 || (deny & !known) != 0 { + return Err(StatusCode::BAD_REQUEST); + } + if (allow & deny) != 0 { + return Err(StatusCode::BAD_REQUEST); + } + Ok(()) +} + +pub async fn set_member_permissions( + State(pool): State, + axum::Extension(auth): axum::Extension, + axum::Extension(notifier): axum::Extension, + axum::extract::Path((team_id, target_user_id)): axum::extract::Path<(Uuid, Uuid)>, + Json(body): Json, +) -> Result { + validate_override_masks(body.allow, body.deny)?; + + let can_manage = crate::permissions::has_team_permission( + &pool, team_id, auth.0, crate::permissions::PERM_MANAGE_MEMBERS, + ) + .await?; + if !can_manage { + return Err(StatusCode::FORBIDDEN); + } + + let is_member = sqlx::query_scalar::<_, bool>( + "SELECT EXISTS(SELECT 1 FROM team_members WHERE team_id = $1 AND user_id = $2)", + ) + .bind(team_id) + .bind(target_user_id) + .fetch_one(&pool) + .await + .map_err(|e| { error!(error = %e, "Failed to check target membership"); StatusCode::INTERNAL_SERVER_ERROR })?; + + if !is_member { + return Err(StatusCode::NOT_FOUND); + } + + let previous = sqlx::query_as::<_, (i64, i64)>( + "SELECT allow_mask, deny_mask FROM team_member_permission_overrides \ + WHERE team_id = $1 AND user_id = $2", + ) + .bind(team_id) + .bind(target_user_id) + .fetch_optional(&pool) + .await + .map_err(|e| { error!(error = %e, "Failed to read existing overrides"); StatusCode::INTERNAL_SERVER_ERROR })? + .unwrap_or((0, 0)); + + if body.allow == 0 && body.deny == 0 { + sqlx::query("DELETE FROM team_member_permission_overrides WHERE team_id = $1 AND user_id = $2") + .bind(team_id) + .bind(target_user_id) + .execute(&pool) + .await + .map_err(|e| { error!(error = %e, "Failed to clear member overrides"); StatusCode::INTERNAL_SERVER_ERROR })?; + } else { + sqlx::query( + "INSERT INTO team_member_permission_overrides \ + (team_id, user_id, allow_mask, deny_mask, updated_at, updated_by) \ + VALUES ($1, $2, $3, $4, now(), $5) \ + ON CONFLICT (team_id, user_id) DO UPDATE \ + SET allow_mask = $3, deny_mask = $4, updated_at = now(), updated_by = $5", + ) + .bind(team_id) + .bind(target_user_id) + .bind(body.allow) + .bind(body.deny) + .bind(auth.0) + .execute(&pool) + .await + .map_err(|e| { error!(error = %e, "Failed to write member overrides"); StatusCode::INTERNAL_SERVER_ERROR })?; + } + + let target_display_name = sqlx::query_scalar::<_, String>("SELECT handle FROM users WHERE id = $1") + .bind(target_user_id) + .fetch_optional(&pool) + .await + .unwrap_or(None); + + info!(team_id = %team_id, target_user_id = %target_user_id, allow = body.allow, deny = body.deny, "Member permission overrides set"); + tokio::spawn(write_audit_event( + pool.clone(), + team_id, + auth.0, + "member.permissions_changed", + Some("user"), + Some(target_user_id.to_string()), + target_display_name, + Some(json!({ + "previous_allow": previous.0, + "previous_deny": previous.1, + "allow": body.allow, + "deny": body.deny, + })), + )); + notify_team_members_changed(&pool, ¬ifier, team_id).await; + Ok(StatusCode::NO_CONTENT) +} + // ─── Invite member (email-based) ────────────────────────────────────────────── #[derive(Deserialize)] @@ -2530,4 +2641,21 @@ mod override_response_tests { assert_eq!(rows, (PERM_CONNECT, PERM_VIEW_SECRETS)); } + + #[test] + fn validates_masks_against_the_known_permission_bits() { + use super::validate_override_masks; + use crate::permissions::ALL_PERMISSIONS; + + assert!(validate_override_masks(PERM_CONNECT, PERM_VIEW_SECRETS).is_ok()); + assert!(validate_override_masks(0, 0).is_ok()); + assert!(validate_override_masks(ALL_PERMISSIONS, 0).is_ok()); + // A bit outside the known set. + assert!(validate_override_masks(1 << 40, 0).is_err()); + assert!(validate_override_masks(0, 1 << 40).is_err()); + // The same bit in both masks is ambiguous. + assert!(validate_override_masks(PERM_CONNECT, PERM_CONNECT).is_err()); + // Negative masks cannot come from a well-formed client. + assert!(validate_override_masks(-1, 0).is_err()); + } } From f2d001ab942522b845bb9d2a38eaee533e145586 Mon Sep 17 00:00:00 2001 From: kipavy Date: Thu, 10 Sep 2026 22:17:56 +0000 Subject: [PATCH 07/19] feat(teams): guard permission-override writes by ownership, self, grant scope and role position --- src/permissions.rs | 8 ++ src/routes/teams.rs | 184 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 192 insertions(+) diff --git a/src/permissions.rs b/src/permissions.rs index 2da3d6d..98509be 100644 --- a/src/permissions.rs +++ b/src/permissions.rs @@ -85,6 +85,14 @@ async fn effective_permissions( }) } +pub async fn effective_permissions_for( + pool: &PgPool, + team_id: Uuid, + user_id: Uuid, +) -> Result { + effective_permissions(pool, team_id, user_id).await +} + /// Returns true if any of (team_id, user_id)'s roles grant `permission`. pub async fn has_team_permission( pool: &PgPool, diff --git a/src/routes/teams.rs b/src/routes/teams.rs index 80e7aff..c942d40 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -1225,6 +1225,66 @@ fn validate_override_masks(allow: i64, deny: i64) -> Result<(), StatusCode> { Ok(()) } +async fn role_position(pool: &PgPool, team_id: Uuid, user_id: Uuid) -> Result, sqlx::Error> { + sqlx::query_scalar::<_, Option>( + "SELECT MIN(tr.position) FROM team_member_roles tmr \ + JOIN team_roles tr ON tr.id = tmr.role_id \ + WHERE tmr.team_id = $1 AND tmr.user_id = $2", + ) + .bind(team_id) + .bind(user_id) + .fetch_one(pool) + .await +} + +async fn override_guardrails( + pool: &PgPool, + team_id: Uuid, + actor_id: Uuid, + target_user_id: Uuid, + allow: i64, +) -> Result<(), StatusCode> { + if actor_id == target_user_id { + return Err(StatusCode::FORBIDDEN); + } + + let target_is_owner = sqlx::query_scalar::<_, bool>( + "SELECT EXISTS( \ + SELECT 1 FROM team_member_roles tmr \ + JOIN team_roles tr ON tr.id = tmr.role_id \ + WHERE tmr.team_id = $1 AND tmr.user_id = $2 \ + AND tr.is_builtin AND tr.name = 'owner')", + ) + .bind(team_id) + .bind(target_user_id) + .fetch_one(pool) + .await + .map_err(|e| { error!(error = %e, "Failed to check target owner role"); StatusCode::INTERNAL_SERVER_ERROR })?; + + if target_is_owner { + return Err(StatusCode::FORBIDDEN); + } + + let actor_effective = crate::permissions::effective_permissions_for(pool, team_id, actor_id).await?; + if (allow & !actor_effective) != 0 { + return Err(StatusCode::FORBIDDEN); + } + + let actor_position = role_position(pool, team_id, actor_id) + .await + .map_err(|e| { error!(error = %e, "Failed to read actor role position"); StatusCode::INTERNAL_SERVER_ERROR })?; + let target_position = role_position(pool, team_id, target_user_id) + .await + .map_err(|e| { error!(error = %e, "Failed to read target role position"); StatusCode::INTERNAL_SERVER_ERROR })?; + + // Position ascends as authority falls; owner is 0. A roleless actor has no position and outranks nobody. + match (actor_position, target_position) { + (Some(actor_pos), Some(target_pos)) if actor_pos < target_pos => Ok(()), + (Some(_), None) => Ok(()), + _ => Err(StatusCode::FORBIDDEN), + } +} + pub async fn set_member_permissions( State(pool): State, axum::Extension(auth): axum::Extension, @@ -1255,6 +1315,8 @@ pub async fn set_member_permissions( return Err(StatusCode::NOT_FOUND); } + override_guardrails(&pool, team_id, auth.0, target_user_id, body.allow).await?; + let previous = sqlx::query_as::<_, (i64, i64)>( "SELECT allow_mask, deny_mask FROM team_member_permission_overrides \ WHERE team_id = $1 AND user_id = $2", @@ -2658,4 +2720,126 @@ mod override_response_tests { // Negative masks cannot come from a well-formed client. assert!(validate_override_masks(-1, 0).is_err()); } + + use super::override_guardrails; + use crate::permissions::{PERM_MANAGE_MEMBERS, PERM_MANAGE_ROLES}; + use crate::test_support::{assign_role, seed_builtin_roles, seed_role}; + use axum::http::StatusCode; + + #[tokio::test] + async fn guardrail_rejects_editing_your_own_overrides() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + add_member(&pool, team, owner).await; + + assert_eq!( + override_guardrails(&pool, team, owner, owner, 0).await.unwrap_err(), + StatusCode::FORBIDDEN + ); + } + + #[tokio::test] + async fn guardrail_rejects_targeting_an_owner() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let manager = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + seed_builtin_roles(&pool, team).await; + + let owner_role = sqlx::query_scalar::<_, uuid::Uuid>( + "SELECT id FROM team_roles WHERE team_id = $1 AND name = 'owner'", + ) + .bind(team) + .fetch_one(&pool) + .await + .unwrap(); + let manager_role = sqlx::query_scalar::<_, uuid::Uuid>( + "SELECT id FROM team_roles WHERE team_id = $1 AND name = 'manager'", + ) + .bind(team) + .fetch_one(&pool) + .await + .unwrap(); + + add_member(&pool, team, owner).await; + assign_role(&pool, team, owner, owner_role).await; + add_member(&pool, team, manager).await; + assign_role(&pool, team, manager, manager_role).await; + + assert_eq!( + override_guardrails(&pool, team, manager, owner, 0).await.unwrap_err(), + StatusCode::FORBIDDEN + ); + } + + #[tokio::test] + async fn guardrail_rejects_granting_a_permission_the_actor_lacks() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let actor = seed_user(&pool).await; + let target = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + + let actor_role = seed_role(&pool, team, "actor", PERM_MANAGE_MEMBERS).await; + let target_role = seed_role(&pool, team, "target", PERM_CONNECT).await; + sqlx::query("UPDATE team_roles SET position = 1 WHERE id = $1").bind(actor_role).execute(&pool).await.unwrap(); + sqlx::query("UPDATE team_roles SET position = 2 WHERE id = $1").bind(target_role).execute(&pool).await.unwrap(); + + add_member(&pool, team, actor).await; + assign_role(&pool, team, actor, actor_role).await; + add_member(&pool, team, target).await; + assign_role(&pool, team, target, target_role).await; + + // The actor does not hold MANAGE_ROLES, so cannot hand it out. + assert_eq!( + override_guardrails(&pool, team, actor, target, PERM_MANAGE_ROLES).await.unwrap_err(), + StatusCode::FORBIDDEN + ); + // Granting something the actor does hold is fine. + assert!(override_guardrails(&pool, team, actor, target, PERM_MANAGE_MEMBERS).await.is_ok()); + } + + #[tokio::test] + async fn guardrail_rejects_targeting_someone_at_or_above_the_actors_position() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let actor = seed_user(&pool).await; + let target = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + + let actor_role = seed_role(&pool, team, "actor", PERM_MANAGE_MEMBERS).await; + let target_role = seed_role(&pool, team, "target", PERM_CONNECT).await; + sqlx::query("UPDATE team_roles SET position = 3 WHERE id = $1").bind(actor_role).execute(&pool).await.unwrap(); + sqlx::query("UPDATE team_roles SET position = 1 WHERE id = $1").bind(target_role).execute(&pool).await.unwrap(); + + add_member(&pool, team, actor).await; + assign_role(&pool, team, actor, actor_role).await; + add_member(&pool, team, target).await; + assign_role(&pool, team, target, target_role).await; + + assert_eq!( + override_guardrails(&pool, team, actor, target, 0).await.unwrap_err(), + StatusCode::FORBIDDEN + ); + } + + #[tokio::test] + async fn guardrail_rejects_an_actor_holding_no_roles() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let actor = seed_user(&pool).await; + let target = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + let target_role = seed_role(&pool, team, "target", PERM_CONNECT).await; + + add_member(&pool, team, actor).await; + add_member(&pool, team, target).await; + assign_role(&pool, team, target, target_role).await; + + assert_eq!( + override_guardrails(&pool, team, actor, target, 0).await.unwrap_err(), + StatusCode::FORBIDDEN + ); + } } From 4e8e4d93a04ec6713172c8cef0cf4963266f3eb5 Mon Sep 17 00:00:00 2001 From: kipavy Date: Thu, 10 Sep 2026 22:20:43 +0000 Subject: [PATCH 08/19] test(teams): pin the self-edit guardrail with a role-holding actor --- src/routes/teams.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/routes/teams.rs b/src/routes/teams.rs index c942d40..d2924a4 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -2731,7 +2731,9 @@ mod override_response_tests { let pool = test_pool_or_skip!(); let owner = seed_user(&pool).await; let team = seed_team(&pool, owner).await; + let role = seed_role(&pool, team, "admin", PERM_MANAGE_MEMBERS).await; add_member(&pool, team, owner).await; + assign_role(&pool, team, owner, role).await; assert_eq!( override_guardrails(&pool, team, owner, owner, 0).await.unwrap_err(), From b61ce3c81df7c025e8a433f4982e83fb2618911c Mon Sep 17 00:00:00 2001 From: kipavy Date: Thu, 10 Sep 2026 22:34:14 +0000 Subject: [PATCH 09/19] test(teams): pin owner immunity independently and cover the roleless-target path --- src/routes/teams.rs | 51 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/src/routes/teams.rs b/src/routes/teams.rs index d2924a4..c2c8682 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -2721,10 +2721,14 @@ mod override_response_tests { assert!(validate_override_masks(-1, 0).is_err()); } - use super::override_guardrails; + use super::{override_guardrails, set_member_permissions, SetMemberPermissionsRequest}; + use crate::auth::AuthUser; use crate::permissions::{PERM_MANAGE_MEMBERS, PERM_MANAGE_ROLES}; + use crate::sync_notifier::SyncNotifier; use crate::test_support::{assign_role, seed_builtin_roles, seed_role}; + use axum::extract::{Path, State}; use axum::http::StatusCode; + use axum::{Extension, Json}; #[tokio::test] async fn guardrail_rejects_editing_your_own_overrides() { @@ -2769,6 +2773,12 @@ mod override_response_tests { add_member(&pool, team, manager).await; assign_role(&pool, team, manager, manager_role).await; + sqlx::query("UPDATE team_roles SET position = 5 WHERE id = $1") + .bind(owner_role) + .execute(&pool) + .await + .unwrap(); + assert_eq!( override_guardrails(&pool, team, manager, owner, 0).await.unwrap_err(), StatusCode::FORBIDDEN @@ -2844,4 +2854,43 @@ mod override_response_tests { StatusCode::FORBIDDEN ); } + + #[tokio::test] + async fn guardrail_allows_editing_a_target_holding_no_roles() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let actor = seed_user(&pool).await; + let target = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + let actor_role = seed_role(&pool, team, "actor", PERM_MANAGE_MEMBERS).await; + + add_member(&pool, team, actor).await; + assign_role(&pool, team, actor, actor_role).await; + add_member(&pool, team, target).await; + + assert!(override_guardrails(&pool, team, actor, target, 0).await.is_ok()); + } + + #[tokio::test] + async fn set_member_permissions_handler_rejects_self_edit() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + let role = seed_role(&pool, team, "admin", PERM_MANAGE_MEMBERS).await; + add_member(&pool, team, owner).await; + assign_role(&pool, team, owner, role).await; + + assert_eq!( + set_member_permissions( + State(pool.clone()), + Extension(AuthUser(owner)), + Extension(SyncNotifier::new()), + Path((team, owner)), + Json(SetMemberPermissionsRequest { allow: 0, deny: 0 }), + ) + .await + .unwrap_err(), + StatusCode::FORBIDDEN + ); + } } From ad217ef460b2b261eb21100e0023d02508e4584e Mon Sep 17 00:00:00 2001 From: kipavy Date: Thu, 10 Sep 2026 22:36:51 +0000 Subject: [PATCH 10/19] test(teams): catch a swapped actor/target at the permissions handler call site --- src/routes/teams.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/routes/teams.rs b/src/routes/teams.rs index c2c8682..f430781 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -2893,4 +2893,36 @@ mod override_response_tests { StatusCode::FORBIDDEN ); } + + #[tokio::test] + async fn set_member_permissions_handler_rejects_a_lower_ranked_caller() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let actor = seed_user(&pool).await; + let target = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + + let actor_role = seed_role(&pool, team, "lower", PERM_MANAGE_MEMBERS).await; + let target_role = seed_role(&pool, team, "higher", PERM_MANAGE_MEMBERS).await; + sqlx::query("UPDATE team_roles SET position = 4 WHERE id = $1").bind(actor_role).execute(&pool).await.unwrap(); + sqlx::query("UPDATE team_roles SET position = 2 WHERE id = $1").bind(target_role).execute(&pool).await.unwrap(); + + add_member(&pool, team, actor).await; + assign_role(&pool, team, actor, actor_role).await; + add_member(&pool, team, target).await; + assign_role(&pool, team, target, target_role).await; + + assert_eq!( + set_member_permissions( + State(pool.clone()), + Extension(AuthUser(actor)), + Extension(SyncNotifier::new()), + Path((team, target)), + Json(SetMemberPermissionsRequest { allow: 0, deny: 0 }), + ) + .await + .unwrap_err(), + StatusCode::FORBIDDEN + ); + } } From a047a207366ca4660445738e33b6e8cf328ed32e Mon Sep 17 00:00:00 2001 From: kipavy Date: Thu, 10 Sep 2026 22:53:52 +0000 Subject: [PATCH 11/19] feat(teams): queue a key rotation when a read-class permission is denied --- src/routes/teams.rs | 133 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 116 insertions(+), 17 deletions(-) diff --git a/src/routes/teams.rs b/src/routes/teams.rs index f430781..81b4787 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -483,6 +483,31 @@ pub async fn add_member( Ok((StatusCode::CREATED, Json(InviteMemberResponse { status: "pending".to_string() }))) } +async fn request_team_rotation( + conn: &mut sqlx::PgConnection, + team_id: Uuid, +) -> Result<(), StatusCode> { + let epoch: i32 = sqlx::query_scalar( + "SELECT COALESCE(MAX(key_version), 1) FROM team_key_epochs WHERE team_id = $1", + ) + .bind(team_id) + .fetch_one(&mut *conn) + .await + .map_err(|e| { error!(error = %e, "Failed to read current epoch for rotation request"); StatusCode::INTERNAL_SERVER_ERROR })?; + + sqlx::query( + "INSERT INTO team_rotation_requests (team_id, requested_at_epoch) VALUES ($1, $2) \ + ON CONFLICT (team_id, requested_at_epoch) DO NOTHING", + ) + .bind(team_id) + .bind(epoch) + .execute(&mut *conn) + .await + .map_err(|e| { error!(error = %e, "Failed to record rotation request"); StatusCode::INTERNAL_SERVER_ERROR })?; + + Ok(()) +} + // ─── Remove member ──────────────────────────────────────────────────────────── pub async fn remove_member( @@ -541,23 +566,7 @@ pub async fn remove_member( .await .map_err(|e| { error!(error = %e, "Failed to remove team vault key"); StatusCode::INTERNAL_SERVER_ERROR })?; - // Remaining members already hold a current-epoch key, so nothing here is - // "under-covered" — flag the removal explicitly so #217 still rotates. - let epoch: i32 = sqlx::query_scalar("SELECT COALESCE(MAX(key_version), 1) FROM team_key_epochs WHERE team_id = $1") - .bind(team_id) - .fetch_one(&mut *tx) - .await - .map_err(|e| { error!(error = %e, "Failed to read current epoch for rotation request"); StatusCode::INTERNAL_SERVER_ERROR })?; - - sqlx::query( - "INSERT INTO team_rotation_requests (team_id, requested_at_epoch) VALUES ($1, $2) \ - ON CONFLICT (team_id, requested_at_epoch) DO NOTHING", - ) - .bind(team_id) - .bind(epoch) - .execute(&mut *tx) - .await - .map_err(|e| { error!(error = %e, "Failed to record rotation request"); StatusCode::INTERNAL_SERVER_ERROR })?; + request_team_rotation(&mut tx, team_id).await?; tx.commit().await.map_err(|e| { error!(error = %e, "Failed to commit remove_member transaction"); @@ -1353,6 +1362,20 @@ pub async fn set_member_permissions( .map_err(|e| { error!(error = %e, "Failed to write member overrides"); StatusCode::INTERNAL_SERVER_ERROR })?; } + const READ_CLASS: i64 = crate::permissions::PERM_VIEW_SECRETS + | crate::permissions::PERM_COPY_SECRETS + | crate::permissions::PERM_CONNECT; + + // Only bits newly denied here trigger a rotation; an unchanged resubmit must not. + let newly_denied = body.deny & !previous.1; + if (newly_denied & READ_CLASS) != 0 { + let mut conn = pool.acquire().await.map_err(|e| { + error!(error = %e, "Failed to acquire connection for rotation request"); + StatusCode::INTERNAL_SERVER_ERROR + })?; + request_team_rotation(&mut conn, team_id).await?; + } + let target_display_name = sqlx::query_scalar::<_, String>("SELECT handle FROM users WHERE id = $1") .bind(target_user_id) .fetch_optional(&pool) @@ -2925,4 +2948,80 @@ mod override_response_tests { StatusCode::FORBIDDEN ); } + + #[tokio::test] + async fn set_member_permissions_handler_queues_rotation_only_for_newly_denied_bits() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let actor = seed_user(&pool).await; + let target = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + + let actor_role = seed_role(&pool, team, "manager", PERM_MANAGE_MEMBERS).await; + add_member(&pool, team, actor).await; + assign_role(&pool, team, actor, actor_role).await; + add_member(&pool, team, target).await; + + let rotations = || async { + sqlx::query_scalar::<_, i64>( + "SELECT COUNT(*) FROM team_rotation_requests WHERE team_id = $1", + ) + .bind(team) + .fetch_one(&pool) + .await + .unwrap() + }; + + set_member_permissions( + State(pool.clone()), + Extension(AuthUser(actor)), + Extension(SyncNotifier::new()), + Path((team, target)), + Json(SetMemberPermissionsRequest { allow: 0, deny: PERM_VIEW_SECRETS }), + ) + .await + .unwrap(); + assert_eq!(rotations().await, 1); + + set_member_permissions( + State(pool.clone()), + Extension(AuthUser(actor)), + Extension(SyncNotifier::new()), + Path((team, target)), + Json(SetMemberPermissionsRequest { allow: 0, deny: PERM_VIEW_SECRETS }), + ) + .await + .unwrap(); + assert_eq!(rotations().await, 1); + } + + #[tokio::test] + async fn denying_a_read_class_bit_records_a_rotation_request() { + let pool = test_pool_or_skip!(); + let team = seed_team(&pool, seed_user(&pool).await).await; + + let mut conn = pool.acquire().await.unwrap(); + super::request_team_rotation(&mut conn, team).await.unwrap(); + + let count: i64 = sqlx::query_scalar( + "SELECT COUNT(*) FROM team_rotation_requests WHERE team_id = $1", + ) + .bind(team) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(count, 1); + + // Idempotent within an epoch. + let mut conn = pool.acquire().await.unwrap(); + super::request_team_rotation(&mut conn, team).await.unwrap(); + let count: i64 = sqlx::query_scalar( + "SELECT COUNT(*) FROM team_rotation_requests WHERE team_id = $1", + ) + .bind(team) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(count, 1); + } } From bda63569965d0606084d9ccd634bc0db7d9579ec Mon Sep 17 00:00:00 2001 From: kipavy Date: Thu, 10 Sep 2026 23:05:42 +0000 Subject: [PATCH 12/19] fix(teams): make the permission write and its rotation request atomic --- src/routes/teams.rs | 57 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/src/routes/teams.rs b/src/routes/teams.rs index 81b4787..d1db83a 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -1326,13 +1326,18 @@ pub async fn set_member_permissions( override_guardrails(&pool, team_id, auth.0, target_user_id, body.allow).await?; + let mut tx = pool.begin().await.map_err(|e| { + error!(error = %e, "Failed to begin set_member_permissions transaction"); + StatusCode::INTERNAL_SERVER_ERROR + })?; + let previous = sqlx::query_as::<_, (i64, i64)>( "SELECT allow_mask, deny_mask FROM team_member_permission_overrides \ WHERE team_id = $1 AND user_id = $2", ) .bind(team_id) .bind(target_user_id) - .fetch_optional(&pool) + .fetch_optional(&mut *tx) .await .map_err(|e| { error!(error = %e, "Failed to read existing overrides"); StatusCode::INTERNAL_SERVER_ERROR })? .unwrap_or((0, 0)); @@ -1341,7 +1346,7 @@ pub async fn set_member_permissions( sqlx::query("DELETE FROM team_member_permission_overrides WHERE team_id = $1 AND user_id = $2") .bind(team_id) .bind(target_user_id) - .execute(&pool) + .execute(&mut *tx) .await .map_err(|e| { error!(error = %e, "Failed to clear member overrides"); StatusCode::INTERNAL_SERVER_ERROR })?; } else { @@ -1357,7 +1362,7 @@ pub async fn set_member_permissions( .bind(body.allow) .bind(body.deny) .bind(auth.0) - .execute(&pool) + .execute(&mut *tx) .await .map_err(|e| { error!(error = %e, "Failed to write member overrides"); StatusCode::INTERNAL_SERVER_ERROR })?; } @@ -1369,13 +1374,14 @@ pub async fn set_member_permissions( // Only bits newly denied here trigger a rotation; an unchanged resubmit must not. let newly_denied = body.deny & !previous.1; if (newly_denied & READ_CLASS) != 0 { - let mut conn = pool.acquire().await.map_err(|e| { - error!(error = %e, "Failed to acquire connection for rotation request"); - StatusCode::INTERNAL_SERVER_ERROR - })?; - request_team_rotation(&mut conn, team_id).await?; + request_team_rotation(&mut tx, team_id).await?; } + tx.commit().await.map_err(|e| { + error!(error = %e, "Failed to commit set_member_permissions transaction"); + StatusCode::INTERNAL_SERVER_ERROR + })?; + let target_display_name = sqlx::query_scalar::<_, String>("SELECT handle FROM users WHERE id = $1") .bind(target_user_id) .fetch_optional(&pool) @@ -2995,6 +3001,41 @@ mod override_response_tests { assert_eq!(rotations().await, 1); } + #[tokio::test] + async fn set_member_permissions_handler_does_not_queue_rotation_for_non_read_class_denies() { + use crate::permissions::PERM_EDIT_CONNECTIONS; + + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let actor = seed_user(&pool).await; + let target = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + + let actor_role = seed_role(&pool, team, "manager", PERM_MANAGE_MEMBERS).await; + add_member(&pool, team, actor).await; + assign_role(&pool, team, actor, actor_role).await; + add_member(&pool, team, target).await; + + set_member_permissions( + State(pool.clone()), + Extension(AuthUser(actor)), + Extension(SyncNotifier::new()), + Path((team, target)), + Json(SetMemberPermissionsRequest { allow: 0, deny: PERM_EDIT_CONNECTIONS }), + ) + .await + .unwrap(); + + let count: i64 = sqlx::query_scalar( + "SELECT COUNT(*) FROM team_rotation_requests WHERE team_id = $1", + ) + .bind(team) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(count, 0); + } + #[tokio::test] async fn denying_a_read_class_bit_records_a_rotation_request() { let pool = test_pool_or_skip!(); From b19a1ecc3300ec83f9f73bd71a196f9afaa3f23f Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 02:25:40 +0000 Subject: [PATCH 13/19] fix(terminal): route vault-session visibility through the permission overlay visible_sessions checked PERM_VIEW_TERMINAL_SESSIONS straight off team_roles.permissions, so an override's deny did nothing and its allow excluded roleless members outright. Replace the inner EXISTS with the same (roleUnion | allow) & ~deny expression the resolvers use. --- src/routes/terminal.rs | 82 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 73 insertions(+), 9 deletions(-) diff --git a/src/routes/terminal.rs b/src/routes/terminal.rs index 676196b..feddb31 100644 --- a/src/routes/terminal.rs +++ b/src/routes/terminal.rs @@ -824,14 +824,17 @@ async fn visible_sessions( FROM terminal_session_vaults tsv JOIN team_members tm ON tm.team_id = tsv.team_id AND tm.user_id = $1 WHERE tsv.session_id = ts.id - AND EXISTS ( - SELECT 1 - FROM team_member_roles tmr_perm - JOIN team_roles tr_perm ON tr_perm.id = tmr_perm.role_id - WHERE tmr_perm.team_id = tsv.team_id - AND tmr_perm.user_id = $1 - AND (tr_perm.permissions & $2) != 0 - ) + AND ( + SELECT (COALESCE(bit_or(tr_perm.permissions), 0) | COALESCE(MAX(o.allow_mask), 0)) + & ~COALESCE(MAX(o.deny_mask), 0) & $2 + FROM team_members tm2 + LEFT JOIN team_member_roles tmr_perm + ON tmr_perm.team_id = tm2.team_id AND tmr_perm.user_id = tm2.user_id + LEFT JOIN team_roles tr_perm ON tr_perm.id = tmr_perm.role_id + LEFT JOIN team_member_permission_overrides o + ON o.team_id = tm2.team_id AND o.user_id = tm2.user_id + WHERE tm2.team_id = tsv.team_id AND tm2.user_id = $1 + ) <> 0 AND ( array_length(ts.allowed_roles, 1) IS NULL OR cardinality(ts.allowed_roles) = 0 @@ -1977,7 +1980,8 @@ mod tests { use crate::rate_limit::RateLimiter; use crate::test_pool_or_skip; use crate::test_support::{ - add_member, default_knock_limiter as knocks, seed_session, seed_team, seed_user, + add_member, assign_role, default_knock_limiter as knocks, seed_role, seed_session, + seed_team, seed_user, set_member_overrides, }; use std::time::Duration; @@ -2272,6 +2276,66 @@ mod tests { .iter().find(|r| r.id == session_id).unwrap().connection_name.is_some()); } + #[tokio::test] + async fn vault_session_visibility_honours_view_terminal_sessions_overrides() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + let role = seed_role(&pool, team, "viewer", crate::permissions::PERM_VIEW_TERMINAL_SESSIONS).await; + + let denied = seed_user(&pool).await; + add_member(&pool, team, denied).await; + assign_role(&pool, team, denied, role).await; + set_member_overrides(&pool, team, denied, 0, crate::permissions::PERM_VIEW_TERMINAL_SESSIONS).await; + + let teammate = seed_user(&pool).await; + add_member(&pool, team, teammate).await; + assign_role(&pool, team, teammate, role).await; + + let host = seed_user(&pool).await; + let session_id = seed_session(&pool, host, "vault").await; + sqlx::query("INSERT INTO terminal_session_vaults (session_id, team_id) VALUES ($1, $2)") + .bind(session_id) + .bind(team) + .execute(&pool) + .await + .unwrap(); + + assert!( + visible_sessions(&pool, denied).await.unwrap().iter().all(|r| r.id != session_id), + "a deny override on VIEW_TERMINAL_SESSIONS must hide the vault session" + ); + assert!( + visible_sessions(&pool, teammate).await.unwrap().iter().any(|r| r.id == session_id), + "a teammate without the override still sees it via their role" + ); + } + + #[tokio::test] + async fn vault_session_visibility_honours_a_roleless_allow_override() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + + let contractor = seed_user(&pool).await; + add_member(&pool, team, contractor).await; + set_member_overrides(&pool, team, contractor, crate::permissions::PERM_VIEW_TERMINAL_SESSIONS, 0).await; + + let host = seed_user(&pool).await; + let session_id = seed_session(&pool, host, "vault").await; + sqlx::query("INSERT INTO terminal_session_vaults (session_id, team_id) VALUES ($1, $2)") + .bind(session_id) + .bind(team) + .execute(&pool) + .await + .unwrap(); + + assert!( + visible_sessions(&pool, contractor).await.unwrap().iter().any(|r| r.id == session_id), + "an allow override must grant visibility even with no role rows" + ); + } + #[tokio::test] async fn admission_stamps_acceptance_once() { let pool = test_pool_or_skip!(); From 2f68fdeeaf5ad9b39ca4568a614f84a5aa182ae3 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 02:26:35 +0000 Subject: [PATCH 14/19] fix(teams): delete member role rows on removal team_member_roles has no FK to team_members, so remove_member left role rows behind; re-inviting restored full role permissions with any deny override gone. Delete the pair's role rows in the same transaction. --- src/routes/teams.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/routes/teams.rs b/src/routes/teams.rs index d1db83a..8800e6f 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -566,6 +566,13 @@ pub async fn remove_member( .await .map_err(|e| { error!(error = %e, "Failed to remove team vault key"); StatusCode::INTERNAL_SERVER_ERROR })?; + sqlx::query("DELETE FROM team_member_roles WHERE team_id = $1 AND user_id = $2") + .bind(team_id) + .bind(user_id) + .execute(&mut *tx) + .await + .map_err(|e| { error!(error = %e, "Failed to remove team member roles"); StatusCode::INTERNAL_SERVER_ERROR })?; + request_team_rotation(&mut tx, team_id).await?; tx.commit().await.map_err(|e| { @@ -2040,6 +2047,34 @@ mod authz_tests { assert_eq!(requested_epoch, 1, "team never rotated before, so the current epoch defaults to 1"); } + #[tokio::test] + async fn remove_member_deletes_their_role_rows_so_re_invite_cannot_restore_them() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let team = seed_team_with_roles(&pool, owner).await; + let victim = member_with_role(&pool, team, PERM_VIEW_SECRETS).await; + + let res = remove_member( + State(pool.clone()), + Extension(AuthUser(owner)), + Extension(SyncNotifier::new()), + Extension(TerminalManager::new()), + Path((team, victim)), + ) + .await; + assert!(res.is_ok(), "remove_member failed: {:?}", res.err()); + + let remaining: i64 = sqlx::query_scalar( + "SELECT COUNT(*) FROM team_member_roles WHERE team_id = $1 AND user_id = $2", + ) + .bind(team) + .bind(victim) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(remaining, 0, "role rows must not survive removal, or re-invite restores them"); + } + #[tokio::test] async fn removing_a_member_revokes_grants_from_a_host_they_no_longer_share_a_team_with() { let pool = test_pool_or_skip!(); From 622dba395cc487475eaede4cb9336e4dc6fe9493 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 02:30:25 +0000 Subject: [PATCH 15/19] fix(teams): trigger key rotation on lost effective read access, not just new denies newly_denied only tracked the deny mask growing, so clearing an allow that was a roleless member's only read-class access queued no rotation at all. Compare effective access before/after instead, and narrow the trigger to the bits get_my_vault_key's CONNECT_OR_VIEW_SECRETS gate actually keys on (VIEW_SECRETS, CONNECT) so rotation isn't queued for denies it can't remediate (e.g. COPY_SECRETS alone). Updated the existing rotation test to give its target a role granting VIEW_SECRETS, so the deny it exercises is a genuine loss under the new comparison. --- src/routes/teams.rs | 100 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 92 insertions(+), 8 deletions(-) diff --git a/src/routes/teams.rs b/src/routes/teams.rs index 8800e6f..23bb526 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -1349,6 +1349,17 @@ pub async fn set_member_permissions( .map_err(|e| { error!(error = %e, "Failed to read existing overrides"); StatusCode::INTERNAL_SERVER_ERROR })? .unwrap_or((0, 0)); + let role_union: i64 = sqlx::query_scalar( + "SELECT COALESCE(bit_or(tr.permissions), 0) FROM team_member_roles tmr \ + JOIN team_roles tr ON tr.id = tmr.role_id \ + WHERE tmr.team_id = $1 AND tmr.user_id = $2", + ) + .bind(team_id) + .bind(target_user_id) + .fetch_one(&mut *tx) + .await + .map_err(|e| { error!(error = %e, "Failed to read target role union"); StatusCode::INTERNAL_SERVER_ERROR })?; + if body.allow == 0 && body.deny == 0 { sqlx::query("DELETE FROM team_member_permission_overrides WHERE team_id = $1 AND user_id = $2") .bind(team_id) @@ -1374,13 +1385,14 @@ pub async fn set_member_permissions( .map_err(|e| { error!(error = %e, "Failed to write member overrides"); StatusCode::INTERNAL_SERVER_ERROR })?; } - const READ_CLASS: i64 = crate::permissions::PERM_VIEW_SECRETS - | crate::permissions::PERM_COPY_SECRETS - | crate::permissions::PERM_CONNECT; + // Bits get_my_vault_key's CONNECT_OR_VIEW_SECRETS gate keys epoch-key access on + // (team_sync.rs) — rotation only helps if it revokes one of these. + const KEY_GATING: i64 = crate::permissions::PERM_VIEW_SECRETS | crate::permissions::PERM_CONNECT; - // Only bits newly denied here trigger a rotation; an unchanged resubmit must not. - let newly_denied = body.deny & !previous.1; - if (newly_denied & READ_CLASS) != 0 { + let prev_effective = (role_union | previous.0) & !previous.1; + let next_effective = (role_union | body.allow) & !body.deny; + let lost = prev_effective & !next_effective & KEY_GATING; + if lost != 0 { request_team_rotation(&mut tx, team_id).await?; } @@ -2789,7 +2801,7 @@ mod override_response_tests { use crate::auth::AuthUser; use crate::permissions::{PERM_MANAGE_MEMBERS, PERM_MANAGE_ROLES}; use crate::sync_notifier::SyncNotifier; - use crate::test_support::{assign_role, seed_builtin_roles, seed_role}; + use crate::test_support::{assign_role, seed_builtin_roles, seed_role, seed_team_with_roles}; use axum::extract::{Path, State}; use axum::http::StatusCode; use axum::{Extension, Json}; @@ -2991,7 +3003,7 @@ mod override_response_tests { } #[tokio::test] - async fn set_member_permissions_handler_queues_rotation_only_for_newly_denied_bits() { + async fn set_member_permissions_handler_queues_rotation_when_a_role_granted_bit_is_denied() { let pool = test_pool_or_skip!(); let owner = seed_user(&pool).await; let actor = seed_user(&pool).await; @@ -2999,9 +3011,13 @@ mod override_response_tests { let team = seed_team(&pool, owner).await; let actor_role = seed_role(&pool, team, "manager", PERM_MANAGE_MEMBERS).await; + sqlx::query("UPDATE team_roles SET position = 1 WHERE id = $1").bind(actor_role).execute(&pool).await.unwrap(); add_member(&pool, team, actor).await; assign_role(&pool, team, actor, actor_role).await; + let target_role = seed_role(&pool, team, "viewer", PERM_VIEW_SECRETS).await; + sqlx::query("UPDATE team_roles SET position = 2 WHERE id = $1").bind(target_role).execute(&pool).await.unwrap(); add_member(&pool, team, target).await; + assign_role(&pool, team, target, target_role).await; let rotations = || async { sqlx::query_scalar::<_, i64>( @@ -3071,6 +3087,74 @@ mod override_response_tests { assert_eq!(count, 0); } + #[tokio::test] + async fn set_member_permissions_handler_does_not_queue_rotation_for_a_copy_secrets_only_deny() { + use crate::permissions::PERM_COPY_SECRETS; + + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let actor = seed_user(&pool).await; + let target = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + + let actor_role = seed_role(&pool, team, "manager", PERM_MANAGE_MEMBERS).await; + sqlx::query("UPDATE team_roles SET position = 1 WHERE id = $1").bind(actor_role).execute(&pool).await.unwrap(); + add_member(&pool, team, actor).await; + assign_role(&pool, team, actor, actor_role).await; + let target_role = seed_role(&pool, team, "copier", PERM_COPY_SECRETS).await; + sqlx::query("UPDATE team_roles SET position = 2 WHERE id = $1").bind(target_role).execute(&pool).await.unwrap(); + add_member(&pool, team, target).await; + assign_role(&pool, team, target, target_role).await; + + set_member_permissions( + State(pool.clone()), + Extension(AuthUser(actor)), + Extension(SyncNotifier::new()), + Path((team, target)), + Json(SetMemberPermissionsRequest { allow: 0, deny: PERM_COPY_SECRETS }), + ) + .await + .unwrap(); + + let count: i64 = sqlx::query_scalar( + "SELECT COUNT(*) FROM team_rotation_requests WHERE team_id = $1", + ) + .bind(team) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(count, 0, "COPY_SECRETS alone does not gate get_my_vault_key, so rotation buys nothing"); + } + + #[tokio::test] + async fn set_member_permissions_handler_queues_rotation_when_clearing_the_only_allow_grant() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let team = seed_team_with_roles(&pool, owner).await; + let contractor = seed_user(&pool).await; + add_member(&pool, team, contractor).await; + crate::test_support::set_member_overrides(&pool, team, contractor, PERM_VIEW_SECRETS, 0).await; + + set_member_permissions( + State(pool.clone()), + Extension(AuthUser(owner)), + Extension(SyncNotifier::new()), + Path((team, contractor)), + Json(SetMemberPermissionsRequest { allow: 0, deny: 0 }), + ) + .await + .unwrap(); + + let count: i64 = sqlx::query_scalar( + "SELECT COUNT(*) FROM team_rotation_requests WHERE team_id = $1", + ) + .bind(team) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(count, 1, "clearing a roleless member's only read-class allow must still revoke key access"); + } + #[tokio::test] async fn denying_a_read_class_bit_records_a_rotation_request() { let pool = test_pool_or_skip!(); From d7a7efe24a1293d0c5ada1768c2decb83957c3b4 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 02:32:16 +0000 Subject: [PATCH 16/19] fix(teams): make override_guardrails' escalation check symmetric MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A target's authority can come entirely from an allow override, so the old check — constraining only bits being granted — let any role-holder with MANAGE_MEMBERS strip an allow-granted capability they never held themselves. Require the actor to also hold every bit they are removing. --- src/routes/teams.rs | 67 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/src/routes/teams.rs b/src/routes/teams.rs index 23bb526..8c3def4 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -1281,8 +1281,21 @@ async fn override_guardrails( return Err(StatusCode::FORBIDDEN); } + let previous_allow: i64 = sqlx::query_scalar( + "SELECT allow_mask FROM team_member_permission_overrides WHERE team_id = $1 AND user_id = $2", + ) + .bind(team_id) + .bind(target_user_id) + .fetch_optional(pool) + .await + .map_err(|e| { error!(error = %e, "Failed to read previous overrides"); StatusCode::INTERNAL_SERVER_ERROR })? + .unwrap_or(0); + let actor_effective = crate::permissions::effective_permissions_for(pool, team_id, actor_id).await?; - if (allow & !actor_effective) != 0 { + // A target's authority can come entirely from an allow override, so the actor + // must hold every bit they revoke, not only every bit they grant. + let removing = previous_allow & !allow; + if ((allow | removing) & !actor_effective) != 0 { return Err(StatusCode::FORBIDDEN); } @@ -2888,6 +2901,58 @@ mod override_response_tests { assert!(override_guardrails(&pool, team, actor, target, PERM_MANAGE_MEMBERS).await.is_ok()); } + #[tokio::test] + async fn set_member_permissions_handler_rejects_stripping_an_allow_override_the_actor_lacks() { + use crate::permissions::PERM_MANAGE_VAULT; + + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let team = seed_team_with_roles(&pool, owner).await; + + // A roleless contractor whose sole authority is an owner-granted allow. + let contractor = seed_user(&pool).await; + add_member(&pool, team, contractor).await; + set_member_overrides(&pool, team, contractor, PERM_MANAGE_VAULT, 0).await; + + let weak_manager_role = seed_role(&pool, team, "weak-manager", PERM_MANAGE_MEMBERS).await; + let weak_manager = seed_user(&pool).await; + add_member(&pool, team, weak_manager).await; + assign_role(&pool, team, weak_manager, weak_manager_role).await; + + assert_eq!( + set_member_permissions( + State(pool.clone()), + Extension(AuthUser(weak_manager)), + Extension(SyncNotifier::new()), + Path((team, contractor)), + Json(SetMemberPermissionsRequest { allow: 0, deny: 0 }), + ) + .await + .unwrap_err(), + StatusCode::FORBIDDEN, + "a manager without MANAGE_VAULT must not be able to strip it via an allow override" + ); + + let strong_manager_role = + seed_role(&pool, team, "strong-manager", PERM_MANAGE_MEMBERS | PERM_MANAGE_VAULT).await; + let strong_manager = seed_user(&pool).await; + add_member(&pool, team, strong_manager).await; + assign_role(&pool, team, strong_manager, strong_manager_role).await; + + assert!( + set_member_permissions( + State(pool.clone()), + Extension(AuthUser(strong_manager)), + Extension(SyncNotifier::new()), + Path((team, contractor)), + Json(SetMemberPermissionsRequest { allow: 0, deny: 0 }), + ) + .await + .is_ok(), + "a manager holding MANAGE_VAULT can strip it" + ); + } + #[tokio::test] async fn guardrail_rejects_targeting_someone_at_or_above_the_actors_position() { let pool = test_pool_or_skip!(); From d9db0bcaafb22b682d16b48525182ba2c90718de Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 02:33:19 +0000 Subject: [PATCH 17/19] refactor(permissions): drop the body-less effective_permissions_for wrapper It differed from the private effective_permissions only in visibility. Make effective_permissions itself pub and update its one call site. --- src/permissions.rs | 10 +--------- src/routes/teams.rs | 2 +- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/permissions.rs b/src/permissions.rs index 98509be..e240422 100644 --- a/src/permissions.rs +++ b/src/permissions.rs @@ -64,7 +64,7 @@ const EFFECTIVE_EXPR: &str = "(COALESCE(bit_or(tr.permissions), 0) | COALESCE(MA & ~COALESCE(MAX(o.deny_mask), 0)"; /// `(roleUnion | allow) & ~deny`. Returns 0 if the user is not a member. -async fn effective_permissions( +pub async fn effective_permissions( pool: &PgPool, team_id: Uuid, user_id: Uuid, @@ -85,14 +85,6 @@ async fn effective_permissions( }) } -pub async fn effective_permissions_for( - pool: &PgPool, - team_id: Uuid, - user_id: Uuid, -) -> Result { - effective_permissions(pool, team_id, user_id).await -} - /// Returns true if any of (team_id, user_id)'s roles grant `permission`. pub async fn has_team_permission( pool: &PgPool, diff --git a/src/routes/teams.rs b/src/routes/teams.rs index 8c3def4..9eb0a8f 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -1291,7 +1291,7 @@ async fn override_guardrails( .map_err(|e| { error!(error = %e, "Failed to read previous overrides"); StatusCode::INTERNAL_SERVER_ERROR })? .unwrap_or(0); - let actor_effective = crate::permissions::effective_permissions_for(pool, team_id, actor_id).await?; + let actor_effective = crate::permissions::effective_permissions(pool, team_id, actor_id).await?; // A target's authority can come entirely from an allow override, so the actor // must hold every bit they revoke, not only every bit they grant. let removing = previous_allow & !allow; From e3eedff95c71cccc9d2abba5e969aa2ee19b7323 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 02:33:37 +0000 Subject: [PATCH 18/19] docs(teams): note why a roleless actor has no position MIN() over zero rows is NULL, not an obvious fact from the match arm alone. --- src/routes/teams.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/routes/teams.rs b/src/routes/teams.rs index 9eb0a8f..7eac40b 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -1306,7 +1306,8 @@ async fn override_guardrails( .await .map_err(|e| { error!(error = %e, "Failed to read target role position"); StatusCode::INTERNAL_SERVER_ERROR })?; - // Position ascends as authority falls; owner is 0. A roleless actor has no position and outranks nobody. + // Position ascends as authority falls; owner is 0. `MIN()` over zero role rows is + // NULL, so a roleless actor has no position and outranks nobody. match (actor_position, target_position) { (Some(actor_pos), Some(target_pos)) if actor_pos < target_pos => Ok(()), (Some(_), None) => Ok(()), From 96450b99d9b8da9e9ad0047e1354d19dfcb0e766 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 08:24:05 +0000 Subject: [PATCH 19/19] fix(teams): rotate only when the vault-key gate is actually crossed The trigger tested bits lost from the read-class mask, but get_my_vault_key gates on Any([CONNECT, VIEW_SECRETS]). Denying VIEW_SECRETS while CONNECT remained therefore queued a rotation the member could still defeat by fetching the new epoch's key, nagging every key-holder client for nothing. --- src/routes/teams.rs | 48 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/src/routes/teams.rs b/src/routes/teams.rs index 7eac40b..dec702f 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -1399,14 +1399,13 @@ pub async fn set_member_permissions( .map_err(|e| { error!(error = %e, "Failed to write member overrides"); StatusCode::INTERNAL_SERVER_ERROR })?; } - // Bits get_my_vault_key's CONNECT_OR_VIEW_SECRETS gate keys epoch-key access on - // (team_sync.rs) — rotation only helps if it revokes one of these. + // Mirrors get_my_vault_key's CONNECT_OR_VIEW_SECRETS gate (team_sync.rs), which is + // Any: only crossing from holding one of these to holding neither revokes key access. const KEY_GATING: i64 = crate::permissions::PERM_VIEW_SECRETS | crate::permissions::PERM_CONNECT; let prev_effective = (role_union | previous.0) & !previous.1; let next_effective = (role_union | body.allow) & !body.deny; - let lost = prev_effective & !next_effective & KEY_GATING; - if lost != 0 { + if (prev_effective & KEY_GATING) != 0 && (next_effective & KEY_GATING) == 0 { request_team_rotation(&mut tx, team_id).await?; } @@ -3153,6 +3152,47 @@ mod override_response_tests { assert_eq!(count, 0); } + #[tokio::test] + async fn set_member_permissions_handler_does_not_queue_rotation_while_connect_still_gates() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let actor = seed_user(&pool).await; + let target = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + + let actor_role = seed_role(&pool, team, "manager", PERM_MANAGE_MEMBERS).await; + sqlx::query("UPDATE team_roles SET position = 1 WHERE id = $1").bind(actor_role).execute(&pool).await.unwrap(); + add_member(&pool, team, actor).await; + assign_role(&pool, team, actor, actor_role).await; + + let target_role = seed_role(&pool, team, "reader", PERM_VIEW_SECRETS | PERM_CONNECT).await; + sqlx::query("UPDATE team_roles SET position = 2 WHERE id = $1").bind(target_role).execute(&pool).await.unwrap(); + add_member(&pool, team, target).await; + assign_role(&pool, team, target, target_role).await; + + set_member_permissions( + State(pool.clone()), + Extension(AuthUser(actor)), + Extension(SyncNotifier::new()), + Path((team, target)), + Json(SetMemberPermissionsRequest { allow: 0, deny: PERM_VIEW_SECRETS }), + ) + .await + .unwrap(); + + let count: i64 = sqlx::query_scalar( + "SELECT COUNT(*) FROM team_rotation_requests WHERE team_id = $1", + ) + .bind(team) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!( + count, 0, + "CONNECT still satisfies the Any key gate, so the member keeps key access and rotation buys nothing" + ); + } + #[tokio::test] async fn set_member_permissions_handler_does_not_queue_rotation_for_a_copy_secrets_only_deny() { use crate::permissions::PERM_COPY_SECRETS;