Signed by hannaSSH key fingerprint: SHA256:4g9cWhkAAw8gwqhJUcVbSnGEvdGwklW+1Aa/fMUu59k
crates/store/src/groups.rs +62 −1
| @@ -107,7 +107,9 @@ impl Store { | |||
| 107 | path: path.to_string(), | 107 | path: path.to_string(), |
| 108 | description: None, | 108 | description: None, |
| 109 | manual: false, | 109 | manual: false, |
| 110 | visibility: Visibility::Private, | 110 | // Auto groups are transparent to the visibility ceiling (see migration |
| 111 | // 0021); only a deliberately-set group restricts the repos beneath it. | ||
| 112 | visibility: Visibility::Public, | ||
| 111 | created_at: now_ms(), | 113 | created_at: now_ms(), |
| 112 | }; | 114 | }; |
| 113 | let sql = "INSERT INTO groups \ | 115 | let sql = "INSERT INTO groups \ |
| @@ -551,6 +553,65 @@ impl Store { | |||
| 551 | Ok(best) | 553 | Ok(best) |
| 552 | } | 554 | } |
| 553 | 555 | ||
| 556 | /// The most-restrictive visibility across a group and its ancestors — the | ||
| 557 | /// ceiling a repository in the group (or a descendant group) may not exceed. | ||
| 558 | /// Auto groups default to public, so they leave the ceiling untouched. | ||
| 559 | /// | ||
| 560 | /// # Errors | ||
| 561 | /// | ||
| 562 | /// Returns [`StoreError::Query`] on a database failure. | ||
| 563 | pub async fn group_visibility_ceiling(&self, group_id: &str) -> Result<Visibility, StoreError> { | ||
| 564 | let mut ceiling = Visibility::Public; | ||
| 565 | let mut next = Some(group_id.to_string()); | ||
| 566 | let mut guard = 0; | ||
| 567 | while let Some(gid) = next { | ||
| 568 | guard += 1; | ||
| 569 | if guard > 64 { | ||
| 570 | break; | ||
| 571 | } | ||
| 572 | let Some(g) = self.group_by_id(&gid).await? else { | ||
| 573 | break; | ||
| 574 | }; | ||
| 575 | if g.visibility.rank() < ceiling.rank() { | ||
| 576 | ceiling = g.visibility; | ||
| 577 | } | ||
| 578 | next = g.parent_id; | ||
| 579 | } | ||
| 580 | Ok(ceiling) | ||
| 581 | } | ||
| 582 | |||
| 583 | /// Clamp every descendant group and repository so none is more visible than | ||
| 584 | /// its group ceiling. Call after lowering a group's visibility. | ||
| 585 | /// | ||
| 586 | /// # Errors | ||
| 587 | /// | ||
| 588 | /// Returns [`StoreError::Query`] on a database failure. | ||
| 589 | pub async fn clamp_subtree_visibility(&self, group: &Group) -> Result<(), StoreError> { | ||
| 590 | let child_prefix = format!("{}/", group.path); | ||
| 591 | // Descendant groups first, top-down (path ASC) so each parent's ceiling is | ||
| 592 | // settled before its children are measured against it. | ||
| 593 | let groups = self.groups_by_owner(&group.owner_id).await?; | ||
| 594 | for g in groups.iter().filter(|g| g.path.starts_with(&child_prefix)) { | ||
| 595 | if let Some(parent_id) = &g.parent_id { | ||
| 596 | let ceiling = self.group_visibility_ceiling(parent_id).await?; | ||
| 597 | if g.visibility.rank() > ceiling.rank() { | ||
| 598 | self.set_group_visibility(&g.id, ceiling).await?; | ||
| 599 | } | ||
| 600 | } | ||
| 601 | } | ||
| 602 | // Then every repository anywhere in the subtree. | ||
| 603 | let repos = self.repos_by_owner(&group.owner_id).await?; | ||
| 604 | for r in repos.iter().filter(|r| r.path.starts_with(&child_prefix)) { | ||
| 605 | if let Some(gid) = &r.group_id { | ||
| 606 | let ceiling = self.group_visibility_ceiling(gid).await?; | ||
| 607 | if r.visibility.rank() > ceiling.rank() { | ||
| 608 | self.set_visibility(&r.id, ceiling).await?; | ||
| 609 | } | ||
| 610 | } | ||
| 611 | } | ||
| 612 | Ok(()) | ||
| 613 | } | ||
| 614 | |||
| 554 | /// Map a `groups` row (selected via [`GROUP_COLUMNS`]) to a [`Group`]. | 615 | /// Map a `groups` row (selected via [`GROUP_COLUMNS`]) to a [`Group`]. |
| 555 | fn map_group(&self, row: &AnyRow) -> Result<Group, sqlx::Error> { | 616 | fn map_group(&self, row: &AnyRow) -> Result<Group, sqlx::Error> { |
| 556 | Ok(Group { | 617 | Ok(Group { |
crates/store/src/tests.rs +63 −0
| @@ -1594,3 +1594,66 @@ async fn rename_group_moves_subtree() { | |||
| 1594 | ); | 1594 | ); |
| 1595 | } | 1595 | } |
| 1596 | } | 1596 | } |
| 1597 | |||
| 1598 | #[tokio::test] | ||
| 1599 | async fn group_visibility_ceiling_clamps_subtree() { | ||
| 1600 | use model::Visibility; | ||
| 1601 | for fx in sqlite_fixtures().await { | ||
| 1602 | let owner = fx | ||
| 1603 | .store | ||
| 1604 | .create_user(sample_user("vis", "vis@example.com")) | ||
| 1605 | .await | ||
| 1606 | .unwrap(); | ||
| 1607 | let org = fx.store.create_group(&owner.id, "org").await.unwrap(); | ||
| 1608 | let repo = repo_in_group(&fx.store, &owner.id, "org/team/app").await; | ||
| 1609 | // Start everything fully public. | ||
| 1610 | fx.store | ||
| 1611 | .set_visibility(&repo.id, Visibility::Public) | ||
| 1612 | .await | ||
| 1613 | .unwrap(); | ||
| 1614 | let team = fx | ||
| 1615 | .store | ||
| 1616 | .group_by_owner_path(&owner.id, "org/team") | ||
| 1617 | .await | ||
| 1618 | .unwrap() | ||
| 1619 | .unwrap(); | ||
| 1620 | fx.store | ||
| 1621 | .set_group_visibility(&team.id, Visibility::Public) | ||
| 1622 | .await | ||
| 1623 | .unwrap(); | ||
| 1624 | assert_eq!( | ||
| 1625 | fx.store.group_visibility_ceiling(&team.id).await.unwrap(), | ||
| 1626 | Visibility::Public | ||
| 1627 | ); | ||
| 1628 | |||
| 1629 | // Lower the top group to internal and cascade the ceiling downward. | ||
| 1630 | fx.store | ||
| 1631 | .set_group_visibility(&org.id, Visibility::Internal) | ||
| 1632 | .await | ||
| 1633 | .unwrap(); | ||
| 1634 | let org = fx.store.group_by_id(&org.id).await.unwrap().unwrap(); | ||
| 1635 | fx.store.clamp_subtree_visibility(&org).await.unwrap(); | ||
| 1636 | |||
| 1637 | let team = fx | ||
| 1638 | .store | ||
| 1639 | .group_by_owner_path(&owner.id, "org/team") | ||
| 1640 | .await | ||
| 1641 | .unwrap() | ||
| 1642 | .unwrap(); | ||
| 1643 | assert_eq!( | ||
| 1644 | team.visibility, | ||
| 1645 | Visibility::Internal, | ||
| 1646 | "subgroup clamped to the new ceiling" | ||
| 1647 | ); | ||
| 1648 | let repo = fx.store.repo_by_id(&repo.id).await.unwrap().unwrap(); | ||
| 1649 | assert_eq!( | ||
| 1650 | repo.visibility, | ||
| 1651 | Visibility::Internal, | ||
| 1652 | "repo clamped to the new ceiling" | ||
| 1653 | ); | ||
| 1654 | assert_eq!( | ||
| 1655 | fx.store.group_visibility_ceiling(&team.id).await.unwrap(), | ||
| 1656 | Visibility::Internal | ||
| 1657 | ); | ||
| 1658 | } | ||
| 1659 | } | ||
crates/web/src/groups.rs +20 −3
| @@ -180,7 +180,15 @@ async fn create_group_from_form( | |||
| 180 | } | 180 | } |
| 181 | other => other.to_string(), | 181 | other => other.to_string(), |
| 182 | })?; | 182 | })?; |
| 183 | if let Some(vis) = Visibility::from_token(&form.visibility) { | 183 | if let Some(mut vis) = Visibility::from_token(&form.visibility) { |
| 184 | // A subgroup may not be more visible than its parent's ceiling. | ||
| 185 | if let Some(parent_id) = &group.parent_id { | ||
| 186 | if let Ok(ceiling) = state.store.group_visibility_ceiling(parent_id).await | ||
| 187 | && vis.rank() > ceiling.rank() | ||
| 188 | { | ||
| 189 | vis = ceiling; | ||
| 190 | } | ||
| 191 | } | ||
| 184 | let _ = state.store.set_group_visibility(&group.id, vis).await; | 192 | let _ = state.store.set_group_visibility(&group.id, vis).await; |
| 185 | } | 193 | } |
| 186 | Ok(group.path) | 194 | Ok(group.path) |
| @@ -399,8 +407,17 @@ pub async fn settings_general( | |||
| 399 | } | 407 | } |
| 400 | other => AppError::from(other), | 408 | other => AppError::from(other), |
| 401 | })?; | 409 | })?; |
| 402 | if let Some(vis) = Visibility::from_token(&form.visibility) { | 410 | if let Some(mut vis) = Visibility::from_token(&form.visibility) { |
| 403 | state.store.set_group_visibility(&group.id, vis).await?; | 411 | // A subgroup may not be more visible than its parent's ceiling. |
| 412 | if let Some(parent_id) = &renamed.parent_id { | ||
| 413 | let ceiling = state.store.group_visibility_ceiling(parent_id).await?; | ||
| 414 | if vis.rank() > ceiling.rank() { | ||
| 415 | vis = ceiling; | ||
| 416 | } | ||
| 417 | } | ||
| 418 | state.store.set_group_visibility(&renamed.id, vis).await?; | ||
| 419 | // Cascade the (possibly lowered) ceiling down to repos and subgroups. | ||
| 420 | state.store.clamp_subtree_visibility(&renamed).await?; | ||
| 404 | } | 421 | } |
| 405 | let _ = owner; | 422 | let _ = owner; |
| 406 | Ok::<String, AppError>(format!("/group-settings/{}", renamed.id)) | 423 | Ok::<String, AppError>(format!("/group-settings/{}", renamed.id)) |
crates/web/src/pages.rs +13 −1
| @@ -427,10 +427,22 @@ async fn create_repo_from_form( | |||
| 427 | b.to_string() | 427 | b.to_string() |
| 428 | } | 428 | } |
| 429 | }; | 429 | }; |
| 430 | let visibility = model::Visibility::from_token(&form.visibility).unwrap_or_else(|| { | 430 | let mut visibility = model::Visibility::from_token(&form.visibility).unwrap_or_else(|| { |
| 431 | model::Visibility::from_token(&state.default_visibility_token()) | 431 | model::Visibility::from_token(&state.default_visibility_token()) |
| 432 | .unwrap_or(model::Visibility::Private) | 432 | .unwrap_or(model::Visibility::Private) |
| 433 | }); | 433 | }); |
| 434 | // A repo may never be more visible than its group (the ceiling walks the | ||
| 435 | // whole ancestor chain; auto groups are public and impose nothing). | ||
| 436 | if let Some(gid) = &group_id { | ||
| 437 | let ceiling = state | ||
| 438 | .store | ||
| 439 | .group_visibility_ceiling(gid) | ||
| 440 | .await | ||
| 441 | .map_err(|e| e.to_string())?; | ||
| 442 | if visibility.rank() > ceiling.rank() { | ||
| 443 | visibility = ceiling; | ||
| 444 | } | ||
| 445 | } | ||
| 434 | let description = { | 446 | let description = { |
| 435 | let d = form.description.trim(); | 447 | let d = form.description.trim(); |
| 436 | (!d.is_empty()).then(|| d.to_string()) | 448 | (!d.is_empty()).then(|| d.to_string()) |
crates/web/src/repo.rs +34 −5
| @@ -1298,9 +1298,18 @@ async fn repo_settings_view( | |||
| 1298 | 1298 | ||
| 1299 | let id = &ctx.repo.id; | 1299 | let id = &ctx.repo.id; |
| 1300 | let vis = ctx.repo.visibility; | 1300 | let vis = ctx.repo.visibility; |
| 1301 | let vis_option = |value: Visibility, label: &str| { | 1301 | // The group ceiling: a repo may not be set more visible than its group. |
| 1302 | html! { | 1302 | let vis_ceiling = match &ctx.repo.group_id { |
| 1303 | option value=(value.as_str()) selected[vis == value] { (label) } | 1303 | Some(gid) => state.store.group_visibility_ceiling(gid).await?, |
| 1304 | None => Visibility::Public, | ||
| 1305 | }; | ||
| 1306 | let vis_option = |value: Visibility, label: &str| -> Markup { | ||
| 1307 | // Hide options above the ceiling, but always keep the current value so the | ||
| 1308 | // form round-trips even if the ceiling later tightened. | ||
| 1309 | if value.rank() <= vis_ceiling.rank() || value == vis { | ||
| 1310 | html! { option value=(value.as_str()) selected[vis == value] { (label) } } | ||
| 1311 | } else { | ||
| 1312 | html! {} | ||
| 1304 | } | 1313 | } |
| 1305 | }; | 1314 | }; |
| 1306 | let body = html! { | 1315 | let body = html! { |
| @@ -1318,6 +1327,12 @@ async fn repo_settings_view( | |||
| 1318 | (vis_option(Visibility::Internal, "Internal — any signed-in user")) | 1327 | (vis_option(Visibility::Internal, "Internal — any signed-in user")) |
| 1319 | (vis_option(Visibility::Private, "Private — only you and collaborators")) | 1328 | (vis_option(Visibility::Private, "Private — only you and collaborators")) |
| 1320 | } | 1329 | } |
| 1330 | @if vis_ceiling.rank() < Visibility::Public.rank() { | ||
| 1331 | p class="muted field-hint" { | ||
| 1332 | "Capped by this repository's group, which is " | ||
| 1333 | (vis_ceiling.as_str()) "." | ||
| 1334 | } | ||
| 1335 | } | ||
| 1321 | label for="object_format" { "Object format" } | 1336 | label for="object_format" { "Object format" } |
| 1322 | select id="object_format" disabled title="Fixed at creation" { | 1337 | select id="object_format" disabled title="Fixed at creation" { |
| 1323 | option { (ctx.repo.object_format.to_uppercase()) } | 1338 | option { (ctx.repo.object_format.to_uppercase()) } |
| @@ -2152,6 +2167,13 @@ pub async fn settings_general( | |||
| 2152 | { | 2167 | { |
| 2153 | vis = parent.visibility; | 2168 | vis = parent.visibility; |
| 2154 | } | 2169 | } |
| 2170 | // Nor more visible than its group ceiling. | ||
| 2171 | if let Some(gid) = &repo.group_id { | ||
| 2172 | let ceiling = state.store.group_visibility_ceiling(gid).await?; | ||
| 2173 | if vis.rank() > ceiling.rank() { | ||
| 2174 | vis = ceiling; | ||
| 2175 | } | ||
| 2176 | } | ||
| 2155 | state.store.set_visibility(&repo.id, vis).await?; | 2177 | state.store.set_visibility(&repo.id, vis).await?; |
| 2156 | } | 2178 | } |
| 2157 | state | 2179 | state |
| @@ -2802,10 +2824,17 @@ async fn group_listing( | |||
| 2802 | 2824 | ||
| 2803 | let all_groups = state.store.groups_by_owner(&owner.id).await?; | 2825 | let all_groups = state.store.groups_by_owner(&owner.id).await?; |
| 2804 | let prefix = format!("{group_path}/"); | 2826 | let prefix = format!("{group_path}/"); |
| 2805 | let subgroups: Vec<_> = all_groups | 2827 | // Direct child groups the viewer may see (a private subgroup is hidden, not |
| 2828 | // just 404 on visit — its very name would otherwise leak here). | ||
| 2829 | let mut subgroups: Vec<&model::Group> = Vec::new(); | ||
| 2830 | for g in all_groups | ||
| 2806 | .iter() | 2831 | .iter() |
| 2807 | .filter(|g| g.path.starts_with(&prefix) && !g.path[prefix.len()..].contains('/')) | 2832 | .filter(|g| g.path.starts_with(&prefix) && !g.path[prefix.len()..].contains('/')) |
| 2808 | .collect(); | 2833 | { |
| 2834 | if crate::groups::group_access(state, viewer.as_ref(), g).await? != auth::Access::None { | ||
| 2835 | subgroups.push(g); | ||
| 2836 | } | ||
| 2837 | } | ||
| 2809 | let repos: Vec<Repo> = visible_repos(state, &owner, viewer.as_ref()) | 2838 | let repos: Vec<Repo> = visible_repos(state, &owner, viewer.as_ref()) |
| 2810 | .await? | 2839 | .await? |
| 2811 | .into_iter() | 2840 | .into_iter() |
migrations/postgres/0021_group_settings.sql +3 −1
| @@ -2,7 +2,9 @@ | |||
| 2 | -- groups are garbage-collected when empty), a visibility, and collaborators | 2 | -- groups are garbage-collected when empty), a visibility, and collaborators |
| 3 | -- whose access cascades to every repository in the group's subtree. | 3 | -- whose access cascades to every repository in the group's subtree. |
| 4 | ALTER TABLE groups ADD COLUMN manual BOOLEAN NOT NULL DEFAULT FALSE; | 4 | ALTER TABLE groups ADD COLUMN manual BOOLEAN NOT NULL DEFAULT FALSE; |
| 5 | ALTER TABLE groups ADD COLUMN visibility TEXT NOT NULL DEFAULT 'private'; | 5 | -- Auto/intermediate groups default to 'public' so they impose no visibility |
| 6 | -- ceiling; only a deliberately-restricted group locks the repos beneath it. | ||
| 7 | ALTER TABLE groups ADD COLUMN visibility TEXT NOT NULL DEFAULT 'public'; | ||
| 6 | 8 | ||
| 7 | CREATE TABLE group_collaborators ( | 9 | CREATE TABLE group_collaborators ( |
| 8 | id TEXT PRIMARY KEY, | 10 | id TEXT PRIMARY KEY, |
migrations/sqlite/0021_group_settings.sql +3 −1
| @@ -2,7 +2,9 @@ | |||
| 2 | -- groups are garbage-collected when empty), a visibility, and collaborators | 2 | -- groups are garbage-collected when empty), a visibility, and collaborators |
| 3 | -- whose access cascades to every repository in the group's subtree. | 3 | -- whose access cascades to every repository in the group's subtree. |
| 4 | ALTER TABLE groups ADD COLUMN manual INTEGER NOT NULL DEFAULT 0; | 4 | ALTER TABLE groups ADD COLUMN manual INTEGER NOT NULL DEFAULT 0; |
| 5 | ALTER TABLE groups ADD COLUMN visibility TEXT NOT NULL DEFAULT 'private'; | 5 | -- Auto/intermediate groups default to 'public' so they impose no visibility |
| 6 | -- ceiling; only a deliberately-restricted group locks the repos beneath it. | ||
| 7 | ALTER TABLE groups ADD COLUMN visibility TEXT NOT NULL DEFAULT 'public'; | ||
| 6 | 8 | ||
| 7 | CREATE TABLE group_collaborators ( | 9 | CREATE TABLE group_collaborators ( |
| 8 | id TEXT PRIMARY KEY, | 10 | id TEXT PRIMARY KEY, |