From 574454cf6126cd953b0b85f23087de134e0a2670 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 10 Nov 2025 23:24:10 -0800 Subject: [PATCH 01/12] clarify permissions so silo.limited_collaborators can promote and demote images --- nexus/src/app/image.rs | 14 +- nexus/tests/integration_tests/images.rs | 326 ++++++++++++++++++++++++ 2 files changed, 336 insertions(+), 4 deletions(-) diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index 9f54d436dbb..703d2eeb289 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -186,10 +186,13 @@ impl super::Nexus { ) -> UpdateResult { match image_lookup { ImageLookup::ProjectImage(lookup) => { + // Modify permission on project image and ListChildren permission + // (as proxy for "has viewer permission") on silo + // are needed to promote an image let (authz_silo, _, authz_project_image, project_image) = lookup.fetch_for(authz::Action::Modify).await?; opctx - .authorize(authz::Action::CreateChild, &authz_silo) + .authorize(authz::Action::ListChildren, &authz_silo) .await?; self.db_datastore .project_image_promote( @@ -215,10 +218,13 @@ impl super::Nexus { ) -> UpdateResult { match image_lookup { ImageLookup::SiloImage(lookup) => { + // Read permission on silo and CreateChild permission on project + // are the two permissions needed to demote an image let (_, authz_silo_image, silo_image) = - lookup.fetch_for(authz::Action::Modify).await?; - let (_, authz_project) = - project_lookup.lookup_for(authz::Action::Modify).await?; + lookup.fetch_for(authz::Action::Read).await?; + let (_, authz_project) = project_lookup + .lookup_for(authz::Action::CreateChild) + .await?; self.db_datastore .silo_image_demote( opctx, diff --git a/nexus/tests/integration_tests/images.rs b/nexus/tests/integration_tests/images.rs index 998a4283b18..248afb664f2 100644 --- a/nexus/tests/integration_tests/images.rs +++ b/nexus/tests/integration_tests/images.rs @@ -574,3 +574,329 @@ async fn test_image_deletion_permissions(cptestctx: &ControlPlaneTestContext) { .await .expect("should be able to delete project image as unpriv user!"); } + +#[nexus_test] +async fn test_limited_collaborator_can_promote_demote_images( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + DiskTest::new(&cptestctx).await; + + // Create a project + create_project(client, PROJECT_NAME).await; + + // Grant the unprivileged user limited-collaborator on the silo + let silo_url = format!("/v1/system/silos/{}", DEFAULT_SILO.id()); + grant_iam( + client, + &silo_url, + SiloRole::LimitedCollaborator, + USER_TEST_UNPRIVILEGED.id(), + AuthnMode::PrivilegedUser, + ) + .await; + + let images_url = get_project_images_url(PROJECT_NAME); + let silo_images_url = "/v1/images"; + + // Create a project image as the unprivileged user (limited-collaborator can do this) + let image_create_params = get_image_create( + params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, + ); + + let image = + NexusRequest::objects_post(client, &images_url, &image_create_params) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::() + .await; + + let image_id = image.identity.id; + + // Verify the image is in the project + let project_images = NexusRequest::object_get(client, &images_url) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(project_images.len(), 1); + assert_eq!(project_images[0].identity.id, image_id); + + // Promote the image to the silo as limited-collaborator + let promote_url = format!("/v1/images/{}/promote", image_id); + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &promote_url) + .expect_status(Some(http::StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::() + .await; + + // Verify the image is now a silo image + let silo_images = NexusRequest::object_get(client, &silo_images_url) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(silo_images.len(), 1); + assert_eq!(silo_images[0].identity.id, image_id); + + // Verify no project images remain + let project_images = NexusRequest::object_get(client, &images_url) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(project_images.len(), 0); + + // Demote the image back to the project as limited-collaborator + let demote_url = + format!("/v1/images/{}/demote?project={}", image_id, PROJECT_NAME); + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &demote_url) + .expect_status(Some(http::StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::() + .await; + + // Verify the image is back in the project + let project_images = NexusRequest::object_get(client, &images_url) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(project_images.len(), 1); + assert_eq!(project_images[0].identity.id, image_id); + + // Verify no silo images remain + let silo_images = NexusRequest::object_get(client, &silo_images_url) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(silo_images.len(), 0); +} + +#[nexus_test] +async fn test_viewer_cannot_promote_demote_images( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + DiskTest::new(&cptestctx).await; + + // Create a project + create_project(client, PROJECT_NAME).await; + + // Grant the unprivileged user viewer on the silo + let silo_url = format!("/v1/system/silos/{}", DEFAULT_SILO.id()); + grant_iam( + client, + &silo_url, + SiloRole::Viewer, + USER_TEST_UNPRIVILEGED.id(), + AuthnMode::PrivilegedUser, + ) + .await; + + let images_url = get_project_images_url(PROJECT_NAME); + + // Create a project image as privileged user + let image_create_params = get_image_create( + params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, + ); + + let image = + NexusRequest::objects_post(client, &images_url, &image_create_params) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap::() + .await; + + let image_id = image.identity.id; + + // Attempt to promote the image as viewer - should fail + let promote_url = format!("/v1/images/{}/promote", image_id); + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &promote_url) + .expect_status(Some(StatusCode::FORBIDDEN)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect("expected viewer to be blocked from promoting image"); + + // Promote the image as privileged user + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &promote_url) + .expect_status(Some(http::StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap::() + .await; + + // Attempt to demote the image as viewer - should fail + let demote_url = + format!("/v1/images/{}/demote?project={}", image_id, PROJECT_NAME); + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &demote_url) + .expect_status(Some(StatusCode::FORBIDDEN)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect("expected viewer to be blocked from demoting image"); +} + +#[nexus_test] +async fn test_project_collaborator_cannot_promote_demote_images( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + DiskTest::new(&cptestctx).await; + + // Create a project + create_project(client, PROJECT_NAME).await; + + // Grant the unprivileged user collaborator on the project only (no silo role) + let project_url = format!("/v1/projects/{}", PROJECT_NAME); + grant_iam( + client, + &project_url, + ProjectRole::Collaborator, + USER_TEST_UNPRIVILEGED.id(), + AuthnMode::PrivilegedUser, + ) + .await; + + let images_url = get_project_images_url(PROJECT_NAME); + + // Create a project image as the unprivileged user (project collaborator can do this) + let image_create_params = get_image_create( + params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, + ); + + let image = + NexusRequest::objects_post(client, &images_url, &image_create_params) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::() + .await; + + let image_id = image.identity.id; + + // Attempt to promote the image as project collaborator - should fail + // (no silo role means they can't ListChildren on silo) + let promote_url = format!("/v1/images/{}/promote", image_id); + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &promote_url) + .expect_status(Some(StatusCode::FORBIDDEN)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect("expected project collaborator to be blocked from promoting image"); + + // Promote the image as privileged user (who has silo role) + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &promote_url) + .expect_status(Some(http::StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap::() + .await; + + // Attempt to demote the image as project collaborator - should fail + // (no silo role means they can't Read silo images) + let demote_url = + format!("/v1/images/{}/demote?project={}", image_id, PROJECT_NAME); + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &demote_url) + .expect_status(Some(StatusCode::FORBIDDEN)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect("expected project collaborator to be blocked from demoting image"); +} + +#[nexus_test] +async fn test_silo_collaborator_can_promote_demote_images( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + DiskTest::new(&cptestctx).await; + + // Create a project + create_project(client, PROJECT_NAME).await; + + // Grant the unprivileged user collaborator on the silo + let silo_url = format!("/v1/system/silos/{}", DEFAULT_SILO.id()); + grant_iam( + client, + &silo_url, + SiloRole::Collaborator, + USER_TEST_UNPRIVILEGED.id(), + AuthnMode::PrivilegedUser, + ) + .await; + + let images_url = get_project_images_url(PROJECT_NAME); + let silo_images_url = "/v1/images"; + + // Create a project image as the unprivileged user (silo collaborator can do this) + let image_create_params = get_image_create( + params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, + ); + + let image = + NexusRequest::objects_post(client, &images_url, &image_create_params) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::() + .await; + + let image_id = image.identity.id; + + // Promote the image to the silo as collaborator + let promote_url = format!("/v1/images/{}/promote", image_id); + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &promote_url) + .expect_status(Some(http::StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::() + .await; + + // Verify the image is now a silo image + let silo_images = NexusRequest::object_get(client, &silo_images_url) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(silo_images.len(), 1); + assert_eq!(silo_images[0].identity.id, image_id); + + // Demote the image back to the project as collaborator + let demote_url = + format!("/v1/images/{}/demote?project={}", image_id, PROJECT_NAME); + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &demote_url) + .expect_status(Some(http::StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::() + .await; + + // Verify the image is back in the project + let project_images = NexusRequest::object_get(client, &images_url) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(project_images.len(), 1); + assert_eq!(project_images[0].identity.id, image_id); +} From 74c7e09102448453ee2a08720724c1a55c2dd706 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 11 Nov 2025 05:18:05 -0800 Subject: [PATCH 02/12] Updates to permissions --- nexus/db-queries/src/db/datastore/image.rs | 6 ++++-- nexus/tests/integration_tests/images.rs | 7 ++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index b48bcf2912b..471c12da3fd 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -173,7 +173,8 @@ impl DataStore { authz_project_image: &authz::ProjectImage, project_image: &ProjectImage, ) -> UpdateResult { - opctx.authorize(authz::Action::CreateChild, authz_silo).await?; + // TODO: This should possiblsy be Modify; exploring Polar change + opctx.authorize(authz::Action::ListChildren, authz_silo).await?; opctx.authorize(authz::Action::Modify, authz_project_image).await?; use nexus_db_schema::schema::image::dsl; @@ -207,7 +208,8 @@ impl DataStore { authz_project: &authz::Project, silo_image: &SiloImage, ) -> UpdateResult { - opctx.authorize(authz::Action::Modify, authz_silo_image).await?; + // TODO: I believe this should be Modify; explore Polar rule change + opctx.authorize(authz::Action::Read, authz_silo_image).await?; opctx.authorize(authz::Action::CreateChild, authz_project).await?; use nexus_db_schema::schema::image::dsl; diff --git a/nexus/tests/integration_tests/images.rs b/nexus/tests/integration_tests/images.rs index 248afb664f2..2afcb1b5bdd 100644 --- a/nexus/tests/integration_tests/images.rs +++ b/nexus/tests/integration_tests/images.rs @@ -599,7 +599,6 @@ async fn test_limited_collaborator_can_promote_demote_images( let images_url = get_project_images_url(PROJECT_NAME); let silo_images_url = "/v1/images"; - // Create a project image as the unprivileged user (limited-collaborator can do this) let image_create_params = get_image_create( params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, ); @@ -788,11 +787,10 @@ async fn test_project_collaborator_cannot_promote_demote_images( let image_id = image.identity.id; // Attempt to promote the image as project collaborator - should fail - // (no silo role means they can't ListChildren on silo) let promote_url = format!("/v1/images/{}/promote", image_id); NexusRequest::new( RequestBuilder::new(client, http::Method::POST, &promote_url) - .expect_status(Some(StatusCode::FORBIDDEN)), + .expect_status(Some(StatusCode::NOT_FOUND)), ) .authn_as(AuthnMode::UnprivilegedUser) .execute() @@ -809,12 +807,11 @@ async fn test_project_collaborator_cannot_promote_demote_images( .await; // Attempt to demote the image as project collaborator - should fail - // (no silo role means they can't Read silo images) let demote_url = format!("/v1/images/{}/demote?project={}", image_id, PROJECT_NAME); NexusRequest::new( RequestBuilder::new(client, http::Method::POST, &demote_url) - .expect_status(Some(StatusCode::FORBIDDEN)), + .expect_status(Some(StatusCode::NOT_FOUND)), ) .authn_as(AuthnMode::UnprivilegedUser) .execute() From 97a01bc40bbc27a91117e600ec1a814b21212c77 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 11 Nov 2025 15:38:53 -0800 Subject: [PATCH 03/12] Use Polar rules --- nexus/auth/src/authz/omicron.polar | 17 ++++++++++++++++- nexus/db-queries/src/db/datastore/image.rs | 6 ++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/nexus/auth/src/authz/omicron.polar b/nexus/auth/src/authz/omicron.polar index ddb5baec8dc..0346badee87 100644 --- a/nexus/auth/src/authz/omicron.polar +++ b/nexus/auth/src/authz/omicron.polar @@ -138,7 +138,10 @@ resource Silo { "list_children" if "viewer"; "read" if "viewer"; - "create_child" if "collaborator"; + # Allow limited-collaborator to create child resources (specifically for + # image promotion). Limited-collaborator is restricted from VPC operations + # but should have full image management capabilities. + "create_child" if "limited-collaborator"; "modify" if "admin"; # Permissions implied by roles on this resource's parent (Fleet). Fleet @@ -825,3 +828,15 @@ resource VpcList { } has_relation(project: Project, "containing_project", collection: VpcList) if collection.project = project; + +# SiloImage modifications for limited-collaborator +# By default, SiloImage uses the InSilo pattern where only "collaborator" can +# modify. We extend this to also allow "limited-collaborator" to modify silo +# images (specifically for demotion). Limited-collaborator is restricted from +# VPC operations but should have full image management capabilities. +# +# Note: If more silo-level resources need limited-collaborator access in the +# future, consider creating InSiloLimited and InSiloFull macro patterns, +# similar to InProjectLimited and InProjectFull. +has_permission(actor: Actor, "modify", silo_image: SiloImage) if + has_role(actor, "limited-collaborator", silo_image.silo); diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index 471c12da3fd..b48bcf2912b 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -173,8 +173,7 @@ impl DataStore { authz_project_image: &authz::ProjectImage, project_image: &ProjectImage, ) -> UpdateResult { - // TODO: This should possiblsy be Modify; exploring Polar change - opctx.authorize(authz::Action::ListChildren, authz_silo).await?; + opctx.authorize(authz::Action::CreateChild, authz_silo).await?; opctx.authorize(authz::Action::Modify, authz_project_image).await?; use nexus_db_schema::schema::image::dsl; @@ -208,8 +207,7 @@ impl DataStore { authz_project: &authz::Project, silo_image: &SiloImage, ) -> UpdateResult { - // TODO: I believe this should be Modify; explore Polar rule change - opctx.authorize(authz::Action::Read, authz_silo_image).await?; + opctx.authorize(authz::Action::Modify, authz_silo_image).await?; opctx.authorize(authz::Action::CreateChild, authz_project).await?; use nexus_db_schema::schema::image::dsl; From 8db9b64250afab3d65fece39859618e07d573671 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 11 Nov 2025 15:53:09 -0800 Subject: [PATCH 04/12] Revert app/image.rs --- nexus/src/app/image.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index 703d2eeb289..9f54d436dbb 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -186,13 +186,10 @@ impl super::Nexus { ) -> UpdateResult { match image_lookup { ImageLookup::ProjectImage(lookup) => { - // Modify permission on project image and ListChildren permission - // (as proxy for "has viewer permission") on silo - // are needed to promote an image let (authz_silo, _, authz_project_image, project_image) = lookup.fetch_for(authz::Action::Modify).await?; opctx - .authorize(authz::Action::ListChildren, &authz_silo) + .authorize(authz::Action::CreateChild, &authz_silo) .await?; self.db_datastore .project_image_promote( @@ -218,13 +215,10 @@ impl super::Nexus { ) -> UpdateResult { match image_lookup { ImageLookup::SiloImage(lookup) => { - // Read permission on silo and CreateChild permission on project - // are the two permissions needed to demote an image let (_, authz_silo_image, silo_image) = - lookup.fetch_for(authz::Action::Read).await?; - let (_, authz_project) = project_lookup - .lookup_for(authz::Action::CreateChild) - .await?; + lookup.fetch_for(authz::Action::Modify).await?; + let (_, authz_project) = + project_lookup.lookup_for(authz::Action::Modify).await?; self.db_datastore .silo_image_demote( opctx, From e5433cf5d50e9a94a207edd55af9524cdcf39d5d Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 11 Nov 2025 17:47:51 -0800 Subject: [PATCH 05/12] Update to using a synthetic resource --- nexus/auth/src/authz/api_resources.rs | 55 +++++++++++++++++++ nexus/auth/src/authz/omicron.polar | 22 ++++++-- nexus/auth/src/authz/oso_generic.rs | 1 + nexus/db-queries/src/db/datastore/image.rs | 6 +- .../src/policy_test/resource_builder.rs | 17 ++++++ nexus/db-queries/src/policy_test/resources.rs | 2 + nexus/db-queries/tests/output/authz-roles.out | 36 +++++++++++- nexus/src/app/image.rs | 12 +++- 8 files changed, 142 insertions(+), 9 deletions(-) diff --git a/nexus/auth/src/authz/api_resources.rs b/nexus/auth/src/authz/api_resources.rs index fdf8af03a05..d1d9c5505ce 100644 --- a/nexus/auth/src/authz/api_resources.rs +++ b/nexus/auth/src/authz/api_resources.rs @@ -819,6 +819,61 @@ impl AuthorizedResource for SiloGroupList { } } +/// Synthetic resource describing the list of Silo Images associated with a Silo +/// +/// This synthetic resource is used to control who can promote project images to +/// silo images. By using a synthetic resource, we can grant limited-collaborators +/// the ability to create silo images (via promotion) without giving them the +/// broader create_child permission on Silo (which would allow creating projects, +/// users, groups, etc.). +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct SiloImageList(Silo); + +impl SiloImageList { + pub fn new(silo: Silo) -> Self { + SiloImageList(silo) + } + + pub fn silo(&self) -> &Silo { + &self.0 + } +} + +impl oso::PolarClass for SiloImageList { + fn get_polar_class_builder() -> oso::ClassBuilder { + oso::Class::builder() + .with_equality_check() + .add_attribute_getter("silo", |list: &SiloImageList| list.0.clone()) + } +} + +impl AuthorizedResource for SiloImageList { + fn load_roles<'fut>( + &'fut self, + opctx: &'fut OpContext, + authn: &'fut authn::Context, + roleset: &'fut mut RoleSet, + ) -> futures::future::BoxFuture<'fut, Result<(), Error>> { + // There are no roles on this resource, but we still need to load the + // Silo-related roles. + self.silo().load_roles(opctx, authn, roleset) + } + + fn on_unauthorized( + &self, + _: &Authz, + error: Error, + _: AnyActor, + _: Action, + ) -> Error { + error + } + + fn polar_class(&self) -> oso::Class { + Self::get_polar_class() + } +} + // Note the session list and the token list have exactly the same behavior /// Synthetic resource for managing a user's sessions diff --git a/nexus/auth/src/authz/omicron.polar b/nexus/auth/src/authz/omicron.polar index 0346badee87..b7a533a13ea 100644 --- a/nexus/auth/src/authz/omicron.polar +++ b/nexus/auth/src/authz/omicron.polar @@ -138,10 +138,7 @@ resource Silo { "list_children" if "viewer"; "read" if "viewer"; - # Allow limited-collaborator to create child resources (specifically for - # image promotion). Limited-collaborator is restricted from VPC operations - # but should have full image management capabilities. - "create_child" if "limited-collaborator"; + "create_child" if "collaborator"; "modify" if "admin"; # Permissions implied by roles on this resource's parent (Fleet). Fleet @@ -829,6 +826,23 @@ resource VpcList { has_relation(project: Project, "containing_project", collection: VpcList) if collection.project = project; +# SiloImageList is a synthetic resource for controlling silo image creation. +# Unlike other silo resources, silo image creation (promotion from project images) +# should be allowed for limited-collaborators, since they need full image management +# capabilities while being restricted from VPC operations. +# This allows organizations to give users full control over images (create, promote, +# demote) while restricting network configuration. +resource SiloImageList { + permissions = [ "list_children", "create_child" ]; + + relations = { containing_silo: Silo }; + + "list_children" if "viewer" on "containing_silo"; + "create_child" if "limited-collaborator" on "containing_silo"; +} +has_relation(silo: Silo, "containing_silo", collection: SiloImageList) + if collection.silo = silo; + # SiloImage modifications for limited-collaborator # By default, SiloImage uses the InSilo pattern where only "collaborator" can # modify. We extend this to also allow "limited-collaborator" to modify silo diff --git a/nexus/auth/src/authz/oso_generic.rs b/nexus/auth/src/authz/oso_generic.rs index b6d627acc33..1af2737fcbd 100644 --- a/nexus/auth/src/authz/oso_generic.rs +++ b/nexus/auth/src/authz/oso_generic.rs @@ -116,6 +116,7 @@ pub fn make_omicron_oso(log: &slog::Logger) -> Result { QuiesceState::get_polar_class(), SiloCertificateList::get_polar_class(), SiloGroupList::get_polar_class(), + SiloImageList::get_polar_class(), SiloIdentityProviderList::get_polar_class(), SiloUserList::get_polar_class(), SiloUserSessionList::get_polar_class(), diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index b48bcf2912b..38ee3ba0883 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -169,11 +169,13 @@ impl DataStore { pub async fn project_image_promote( &self, opctx: &OpContext, - authz_silo: &authz::Silo, + _authz_silo: &authz::Silo, authz_project_image: &authz::ProjectImage, project_image: &ProjectImage, ) -> UpdateResult { - opctx.authorize(authz::Action::CreateChild, authz_silo).await?; + // Authorization for creating silo images is checked in the app layer + // via SiloImageList to allow limited-collaborators to promote without + // granting them broad create_child on Silo. opctx.authorize(authz::Action::Modify, authz_project_image).await?; use nexus_db_schema::schema::image::dsl; diff --git a/nexus/db-queries/src/policy_test/resource_builder.rs b/nexus/db-queries/src/policy_test/resource_builder.rs index d6dd1f2721b..6872d0f1797 100644 --- a/nexus/db-queries/src/policy_test/resource_builder.rs +++ b/nexus/db-queries/src/policy_test/resource_builder.rs @@ -435,3 +435,20 @@ impl DynAuthorizedResource for authz::VpcList { format!("{}: vpc list", self.project().resource_name()) } } + +impl DynAuthorizedResource for authz::SiloImageList { + fn do_authorize<'a, 'b>( + &'a self, + opctx: &'b OpContext, + action: authz::Action, + ) -> BoxFuture<'a, Result<(), Error>> + where + 'b: 'a, + { + opctx.authorize(action, self).boxed() + } + + fn resource_name(&self) -> String { + format!("{}: silo image list", self.silo().resource_name()) + } +} diff --git a/nexus/db-queries/src/policy_test/resources.rs b/nexus/db-queries/src/policy_test/resources.rs index f8a6f2890e2..cdf42758eb1 100644 --- a/nexus/db-queries/src/policy_test/resources.rs +++ b/nexus/db-queries/src/policy_test/resources.rs @@ -245,6 +245,8 @@ async fn make_silo( LookupType::ByName(format!("{}-certificate", silo_name)), )); + builder.new_resource(authz::SiloImageList::new(silo.clone())); + builder.new_resource(authz::SiloIdentityProviderList::new(silo.clone())); let idp_id = Uuid::new_v4(); builder.new_resource(authz::IdentityProvider::new( diff --git a/nexus/db-queries/tests/output/authz-roles.out b/nexus/db-queries/tests/output/authz-roles.out index c11031b873a..861c409b8d9 100644 --- a/nexus/db-queries/tests/output/authz-roles.out +++ b/nexus/db-queries/tests/output/authz-roles.out @@ -270,6 +270,23 @@ resource: Certificate "silo1-certificate" unauthenticated ! ! ! ! ! ! ! ! scim ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ +resource: Silo "silo1": silo image list + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✘ ✔ ✘ ✘ ✘ ✔ ✘ + silo1-collaborator ✘ ✘ ✔ ✘ ✘ ✘ ✔ ✘ + silo1-limited-collaborator ✘ ✘ ✔ ✘ ✘ ✘ ✔ ✘ + silo1-viewer ✘ ✘ ✔ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-limited-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + scim ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + resource: Silo "silo1": identity provider list USER Q R LC RP M MP CC D @@ -414,7 +431,7 @@ resource: SiloImage "silo1-silo-image" fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ silo1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ silo1-collaborator ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ - silo1-limited-collaborator ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ + silo1-limited-collaborator ✘ ✔ ✔ ✔ ✔ ✔ ✘ ✔ silo1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ @@ -1069,6 +1086,23 @@ resource: Certificate "silo2-certificate" unauthenticated ! ! ! ! ! ! ! ! scim ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ +resource: Silo "silo2": silo image list + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-limited-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-limited-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + scim ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + resource: Silo "silo2": identity provider list USER Q R LC RP M MP CC D diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index 9f54d436dbb..ce38fabb7fa 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -188,9 +188,15 @@ impl super::Nexus { ImageLookup::ProjectImage(lookup) => { let (authz_silo, _, authz_project_image, project_image) = lookup.fetch_for(authz::Action::Modify).await?; + + // Check if the user can create silo images (promote from project images). + // We use SiloImageList to allow limited-collaborators to promote images + // without granting them the broader create_child permission on Silo. + let authz_silo_image_list = authz::SiloImageList::new(authz_silo.clone()); opctx - .authorize(authz::Action::CreateChild, &authz_silo) + .authorize(authz::Action::CreateChild, &authz_silo_image_list) .await?; + self.db_datastore .project_image_promote( opctx, @@ -217,8 +223,10 @@ impl super::Nexus { ImageLookup::SiloImage(lookup) => { let (_, authz_silo_image, silo_image) = lookup.fetch_for(authz::Action::Modify).await?; + // Check CreateChild on the project since we're creating a ProjectImage. + // This allows limited-collaborators to demote images. let (_, authz_project) = - project_lookup.lookup_for(authz::Action::Modify).await?; + project_lookup.lookup_for(authz::Action::CreateChild).await?; self.db_datastore .silo_image_demote( opctx, From d30e9bc32295504af4dc3f460aa0d5b78f713c59 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 12 Nov 2025 13:03:25 -0800 Subject: [PATCH 06/12] update tests --- nexus/tests/integration_tests/images.rs | 135 ++++++++++++++++++------ 1 file changed, 102 insertions(+), 33 deletions(-) diff --git a/nexus/tests/integration_tests/images.rs b/nexus/tests/integration_tests/images.rs index 2afcb1b5bdd..4212c057a3a 100644 --- a/nexus/tests/integration_tests/images.rs +++ b/nexus/tests/integration_tests/images.rs @@ -575,6 +575,85 @@ async fn test_image_deletion_permissions(cptestctx: &ControlPlaneTestContext) { .expect("should be able to delete project image as unpriv user!"); } +#[nexus_test] +async fn test_silo_collaborator_can_promote_demote_images( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + DiskTest::new(&cptestctx).await; + + // Create a project + create_project(client, PROJECT_NAME).await; + + // Grant the unprivileged user collaborator on the silo + let silo_url = format!("/v1/system/silos/{}", DEFAULT_SILO.id()); + grant_iam( + client, + &silo_url, + SiloRole::Collaborator, + USER_TEST_UNPRIVILEGED.id(), + AuthnMode::PrivilegedUser, + ) + .await; + + let images_url = get_project_images_url(PROJECT_NAME); + let silo_images_url = "/v1/images"; + + // Create a project image as the unprivileged user (silo collaborator can do this) + let image_create_params = get_image_create( + params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, + ); + + let image = + NexusRequest::objects_post(client, &images_url, &image_create_params) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::() + .await; + + let image_id = image.identity.id; + + // Promote the image to the silo as collaborator + let promote_url = format!("/v1/images/{}/promote", image_id); + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &promote_url) + .expect_status(Some(http::StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::() + .await; + + // Verify the image is now a silo image + let silo_images = NexusRequest::object_get(client, &silo_images_url) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(silo_images.len(), 1); + assert_eq!(silo_images[0].identity.id, image_id); + + // Demote the image back to the project as collaborator + let demote_url = + format!("/v1/images/{}/demote?project={}", image_id, PROJECT_NAME); + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &demote_url) + .expect_status(Some(http::StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::() + .await; + + // Verify the image is back in the project + let project_images = NexusRequest::object_get(client, &images_url) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(project_images.len(), 1); + assert_eq!(project_images[0].identity.id, image_id); +} + #[nexus_test] async fn test_limited_collaborator_can_promote_demote_images( cptestctx: &ControlPlaneTestContext, @@ -820,7 +899,7 @@ async fn test_project_collaborator_cannot_promote_demote_images( } #[nexus_test] -async fn test_silo_collaborator_can_promote_demote_images( +async fn test_project_limited_collaborator_cannot_promote_demote_images( cptestctx: &ControlPlaneTestContext, ) { let client = &cptestctx.external_client; @@ -829,21 +908,20 @@ async fn test_silo_collaborator_can_promote_demote_images( // Create a project create_project(client, PROJECT_NAME).await; - // Grant the unprivileged user collaborator on the silo - let silo_url = format!("/v1/system/silos/{}", DEFAULT_SILO.id()); + // Grant the unprivileged user limited-collaborator on the project only (no silo role) + let project_url = format!("/v1/projects/{}", PROJECT_NAME); grant_iam( client, - &silo_url, - SiloRole::Collaborator, + &project_url, + ProjectRole::LimitedCollaborator, USER_TEST_UNPRIVILEGED.id(), AuthnMode::PrivilegedUser, ) .await; let images_url = get_project_images_url(PROJECT_NAME); - let silo_images_url = "/v1/images"; - // Create a project image as the unprivileged user (silo collaborator can do this) + // Create a project image as the unprivileged user (project limited-collaborator can do this) let image_create_params = get_image_create( params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, ); @@ -856,44 +934,35 @@ async fn test_silo_collaborator_can_promote_demote_images( let image_id = image.identity.id; - // Promote the image to the silo as collaborator + // Attempt to promote the image as project limited-collaborator - should fail let promote_url = format!("/v1/images/{}/promote", image_id); NexusRequest::new( RequestBuilder::new(client, http::Method::POST, &promote_url) - .expect_status(Some(http::StatusCode::ACCEPTED)), + .expect_status(Some(StatusCode::NOT_FOUND)), ) .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect("expected project limited-collaborator to be blocked from promoting image"); + + // Promote the image as privileged user (who has silo role) + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &promote_url) + .expect_status(Some(http::StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::PrivilegedUser) .execute_and_parse_unwrap::() .await; - // Verify the image is now a silo image - let silo_images = NexusRequest::object_get(client, &silo_images_url) - .authn_as(AuthnMode::UnprivilegedUser) - .execute_and_parse_unwrap::>() - .await - .items; - - assert_eq!(silo_images.len(), 1); - assert_eq!(silo_images[0].identity.id, image_id); - - // Demote the image back to the project as collaborator + // Attempt to demote the image as project limited-collaborator - should fail let demote_url = format!("/v1/images/{}/demote?project={}", image_id, PROJECT_NAME); NexusRequest::new( RequestBuilder::new(client, http::Method::POST, &demote_url) - .expect_status(Some(http::StatusCode::ACCEPTED)), + .expect_status(Some(StatusCode::NOT_FOUND)), ) .authn_as(AuthnMode::UnprivilegedUser) - .execute_and_parse_unwrap::() - .await; - - // Verify the image is back in the project - let project_images = NexusRequest::object_get(client, &images_url) - .authn_as(AuthnMode::UnprivilegedUser) - .execute_and_parse_unwrap::>() - .await - .items; - - assert_eq!(project_images.len(), 1); - assert_eq!(project_images[0].identity.id, image_id); + .execute() + .await + .expect("expected project limited-collaborator to be blocked from demoting image"); } From 35e49683ec8c36eedcc986886d5de0b5f05d0014 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 12 Nov 2025 14:24:40 -0800 Subject: [PATCH 07/12] cargo fmt --- nexus/src/app/image.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index ce38fabb7fa..1f6eb02dd1d 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -192,9 +192,13 @@ impl super::Nexus { // Check if the user can create silo images (promote from project images). // We use SiloImageList to allow limited-collaborators to promote images // without granting them the broader create_child permission on Silo. - let authz_silo_image_list = authz::SiloImageList::new(authz_silo.clone()); + let authz_silo_image_list = + authz::SiloImageList::new(authz_silo.clone()); opctx - .authorize(authz::Action::CreateChild, &authz_silo_image_list) + .authorize( + authz::Action::CreateChild, + &authz_silo_image_list, + ) .await?; self.db_datastore @@ -225,8 +229,9 @@ impl super::Nexus { lookup.fetch_for(authz::Action::Modify).await?; // Check CreateChild on the project since we're creating a ProjectImage. // This allows limited-collaborators to demote images. - let (_, authz_project) = - project_lookup.lookup_for(authz::Action::CreateChild).await?; + let (_, authz_project) = project_lookup + .lookup_for(authz::Action::CreateChild) + .await?; self.db_datastore .silo_image_demote( opctx, From a9e7fd0c566a8c2d1f1d76d618593cecde646d42 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 12 Nov 2025 15:23:33 -0800 Subject: [PATCH 08/12] Update test names --- nexus/tests/integration_tests/images.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nexus/tests/integration_tests/images.rs b/nexus/tests/integration_tests/images.rs index 4212c057a3a..e89bcffd3ec 100644 --- a/nexus/tests/integration_tests/images.rs +++ b/nexus/tests/integration_tests/images.rs @@ -655,7 +655,7 @@ async fn test_silo_collaborator_can_promote_demote_images( } #[nexus_test] -async fn test_limited_collaborator_can_promote_demote_images( +async fn test_silo_limited_collaborator_can_promote_demote_images( cptestctx: &ControlPlaneTestContext, ) { let client = &cptestctx.external_client; @@ -761,7 +761,7 @@ async fn test_limited_collaborator_can_promote_demote_images( } #[nexus_test] -async fn test_viewer_cannot_promote_demote_images( +async fn test_silo_viewer_cannot_promote_demote_images( cptestctx: &ControlPlaneTestContext, ) { let client = &cptestctx.external_client; From bb90c8066c4234215e2b6a60c4abcd96e9056f73 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Thu, 13 Nov 2025 14:09:04 -0800 Subject: [PATCH 09/12] add tests re: silo image deletion --- nexus/tests/integration_tests/images.rs | 108 ++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/nexus/tests/integration_tests/images.rs b/nexus/tests/integration_tests/images.rs index e89bcffd3ec..03d5fe30d47 100644 --- a/nexus/tests/integration_tests/images.rs +++ b/nexus/tests/integration_tests/images.rs @@ -652,6 +652,37 @@ async fn test_silo_collaborator_can_promote_demote_images( assert_eq!(project_images.len(), 1); assert_eq!(project_images[0].identity.id, image_id); + + // Test that silo collaborator can delete silo images + // First, promote the image back to silo + let promote_url = format!("/v1/images/{}/promote", image_id); + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &promote_url) + .expect_status(Some(http::StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::() + .await; + + // Delete the silo image as collaborator - should succeed + let image_url = format!("/v1/images/{}", image_id); + NexusRequest::new( + RequestBuilder::new(client, http::Method::DELETE, &image_url) + .expect_status(Some(http::StatusCode::NO_CONTENT)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect("silo collaborator should be able to delete silo image"); + + // Verify no silo images remain + let silo_images = NexusRequest::object_get(client, &silo_images_url) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(silo_images.len(), 0); } #[nexus_test] @@ -758,6 +789,46 @@ async fn test_silo_limited_collaborator_can_promote_demote_images( .items; assert_eq!(silo_images.len(), 0); + + // Test that limited-collaborator can delete both project and silo images + // First, promote the image back to silo + let promote_url = format!("/v1/images/{}/promote", image_id); + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &promote_url) + .expect_status(Some(http::StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::() + .await; + + // Verify it's a silo image + let silo_images = NexusRequest::object_get(client, &silo_images_url) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(silo_images.len(), 1); + + // Delete the silo image as limited-collaborator - should succeed + let image_url = format!("/v1/images/{}", image_id); + NexusRequest::new( + RequestBuilder::new(client, http::Method::DELETE, &image_url) + .expect_status(Some(http::StatusCode::NO_CONTENT)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect("limited-collaborator should be able to delete silo image"); + + // Verify no silo images remain + let silo_images = NexusRequest::object_get(client, &silo_images_url) + .authn_as(AuthnMode::UnprivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(silo_images.len(), 0); } #[nexus_test] @@ -827,6 +898,17 @@ async fn test_silo_viewer_cannot_promote_demote_images( .execute() .await .expect("expected viewer to be blocked from demoting image"); + + // Attempt to delete the silo image as viewer - should fail + let image_url = format!("/v1/images/{}", image_id); + NexusRequest::new( + RequestBuilder::new(client, http::Method::DELETE, &image_url) + .expect_status(Some(StatusCode::FORBIDDEN)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect("expected viewer to be blocked from deleting silo image"); } #[nexus_test] @@ -896,6 +978,20 @@ async fn test_project_collaborator_cannot_promote_demote_images( .execute() .await .expect("expected project collaborator to be blocked from demoting image"); + + // Attempt to delete the silo image as project collaborator - should fail + // Project collaborators have no silo-level permissions, so they get NOT_FOUND + let image_url = format!("/v1/images/{}", image_id); + NexusRequest::new( + RequestBuilder::new(client, http::Method::DELETE, &image_url) + .expect_status(Some(StatusCode::NOT_FOUND)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect( + "expected project collaborator to be blocked from deleting silo image", + ); } #[nexus_test] @@ -965,4 +1061,16 @@ async fn test_project_limited_collaborator_cannot_promote_demote_images( .execute() .await .expect("expected project limited-collaborator to be blocked from demoting image"); + + // Attempt to delete the silo image as project limited-collaborator - should fail + // Project limited-collaborators have no silo-level permissions, so they get NOT_FOUND + let image_url = format!("/v1/images/{}", image_id); + NexusRequest::new( + RequestBuilder::new(client, http::Method::DELETE, &image_url) + .expect_status(Some(StatusCode::NOT_FOUND)), + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect("expected project limited-collaborator to be blocked from deleting silo image"); } From adb8093b3086f1bcc9dff0cbfcac8bd96d5aab0e Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 14 Nov 2025 15:25:07 -0800 Subject: [PATCH 10/12] move authz check back to datastore from app layer --- nexus/db-queries/src/db/datastore/image.rs | 11 +++++++---- nexus/src/app/image.rs | 14 ++++---------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index 38ee3ba0883..a72b7c83918 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -169,13 +169,16 @@ impl DataStore { pub async fn project_image_promote( &self, opctx: &OpContext, - _authz_silo: &authz::Silo, + authz_silo_image_list: &authz::SiloImageList, authz_project_image: &authz::ProjectImage, project_image: &ProjectImage, ) -> UpdateResult { - // Authorization for creating silo images is checked in the app layer - // via SiloImageList to allow limited-collaborators to promote without - // granting them broad create_child on Silo. + // Check if the user can create silo images (promote from project images). + // We use SiloImageList to allow limited-collaborators to promote images + // without granting them the broader create_child permission on Silo. + opctx + .authorize(authz::Action::CreateChild, authz_silo_image_list) + .await?; opctx.authorize(authz::Action::Modify, authz_project_image).await?; use nexus_db_schema::schema::image::dsl; diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index 1f6eb02dd1d..f9da662d957 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -189,22 +189,16 @@ impl super::Nexus { let (authz_silo, _, authz_project_image, project_image) = lookup.fetch_for(authz::Action::Modify).await?; - // Check if the user can create silo images (promote from project images). - // We use SiloImageList to allow limited-collaborators to promote images - // without granting them the broader create_child permission on Silo. + // Construct SiloImageList for the authorization check in the datastore. + // This allows limited-collaborators to promote images without granting + // them the broader create_child permission on Silo. let authz_silo_image_list = authz::SiloImageList::new(authz_silo.clone()); - opctx - .authorize( - authz::Action::CreateChild, - &authz_silo_image_list, - ) - .await?; self.db_datastore .project_image_promote( opctx, - &authz_silo, + &authz_silo_image_list, &authz_project_image, &project_image, ) From a5d82ab8acf6c9d079818daf2219bf0bd8d7160a Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 14 Nov 2025 15:33:02 -0800 Subject: [PATCH 11/12] construct silo image list at datastore --- nexus/db-queries/src/db/datastore/image.rs | 5 +++-- nexus/src/app/image.rs | 8 +------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index a72b7c83918..e80c3b104fd 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -169,15 +169,16 @@ impl DataStore { pub async fn project_image_promote( &self, opctx: &OpContext, - authz_silo_image_list: &authz::SiloImageList, + authz_silo: &authz::Silo, authz_project_image: &authz::ProjectImage, project_image: &ProjectImage, ) -> UpdateResult { // Check if the user can create silo images (promote from project images). // We use SiloImageList to allow limited-collaborators to promote images // without granting them the broader create_child permission on Silo. + let authz_silo_image_list = authz::SiloImageList::new(authz_silo.clone()); opctx - .authorize(authz::Action::CreateChild, authz_silo_image_list) + .authorize(authz::Action::CreateChild, &authz_silo_image_list) .await?; opctx.authorize(authz::Action::Modify, authz_project_image).await?; diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index f9da662d957..578b7e7ba88 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -189,16 +189,10 @@ impl super::Nexus { let (authz_silo, _, authz_project_image, project_image) = lookup.fetch_for(authz::Action::Modify).await?; - // Construct SiloImageList for the authorization check in the datastore. - // This allows limited-collaborators to promote images without granting - // them the broader create_child permission on Silo. - let authz_silo_image_list = - authz::SiloImageList::new(authz_silo.clone()); - self.db_datastore .project_image_promote( opctx, - &authz_silo_image_list, + &authz_silo, &authz_project_image, &project_image, ) From 81967762236ae2dceb0242ae334e4b938b991f14 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 14 Nov 2025 15:34:57 -0800 Subject: [PATCH 12/12] cargo fmt --- nexus/db-queries/src/db/datastore/image.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index e80c3b104fd..9bc39595e68 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -176,7 +176,8 @@ impl DataStore { // Check if the user can create silo images (promote from project images). // We use SiloImageList to allow limited-collaborators to promote images // without granting them the broader create_child permission on Silo. - let authz_silo_image_list = authz::SiloImageList::new(authz_silo.clone()); + let authz_silo_image_list = + authz::SiloImageList::new(authz_silo.clone()); opctx .authorize(authz::Action::CreateChild, &authz_silo_image_list) .await?;