From 18f3e669254e663cd97866d01a89e1bd909fab8c Mon Sep 17 00:00:00 2001 From: Alon Gubkin Date: Tue, 28 Jul 2026 17:38:50 +0300 Subject: [PATCH] fix: allow legacy observe grant removal Existing prepared stacks contain the account-wide observe grant that older managers inserted automatically. The opt-in observation change correctly stops generating it, but compatibility checks then reject every update as permission drift. Ignore only that exact named grant on the old management side. Additions, named profiles, inline permission sets, other grants, and permission modes remain protected. Refs ALIEN-388 --- .../permission_profiles_unchanged.rs | 145 +++++++++++++++++- .../management_permission_profile.rs | 2 +- 2 files changed, 142 insertions(+), 5 deletions(-) diff --git a/crates/alien-preflights/src/compatibility/permission_profiles_unchanged.rs b/crates/alien-preflights/src/compatibility/permission_profiles_unchanged.rs index 64854d34e..e47d05e6c 100644 --- a/crates/alien-preflights/src/compatibility/permission_profiles_unchanged.rs +++ b/crates/alien-preflights/src/compatibility/permission_profiles_unchanged.rs @@ -1,5 +1,7 @@ use crate::error::Result; -use crate::mutations::management_permission_profile::GATE_DERIVED_GLOBAL_SUFFIXES; +use crate::mutations::management_permission_profile::{ + GATE_DERIVED_GLOBAL_SUFFIXES, OBSERVE_PERMISSION_SET_ID, +}; use crate::{CheckResult, StackCompatibilityCheck}; use alien_core::permissions::{ManagementPermissions, PermissionProfile, PermissionSetReference}; use alien_core::Stack; @@ -60,15 +62,32 @@ fn gated_contributions(old_stack: &Stack, new_stack: &Stack) -> GatedContributio /// data-capable grant here would let it slip through as soon as any resource /// of its type is gated. Full references are compared (not their ids), so an /// inline set whose body changed under a stable id still reads as drift. +/// +/// Older managers also inserted the named `observe/observe` grant into every +/// prepared management profile. Application stacks no longer receive that +/// account-wide grant. Compatibility may therefore omit that exact named +/// reference from the old management profile only; named profiles, inline +/// permission sets, and additions on the new side remain unchanged. fn without_gate_derived<'a>( grants: Option<&'a Vec>, gated: &GatedContributions, + omit_legacy_automatic_observe: bool, ) -> Vec<&'a PermissionSetReference> { grants .map(|grants| { grants .iter() .filter(|grant| { + if omit_legacy_automatic_observe + && matches!( + grant, + PermissionSetReference::Name(name) + if name == OBSERVE_PERMISSION_SET_ID + ) + { + return false; + } + !gated.type_prefixes.iter().any(|prefix| { grant .id() @@ -92,6 +111,7 @@ fn profiles_differ_outside_gates( old_profile: &PermissionProfile, new_profile: &PermissionProfile, gated: &GatedContributions, + allow_legacy_automatic_observe_removal: bool, ) -> bool { let scopes: HashSet<&str> = old_profile .0 @@ -105,7 +125,9 @@ fn profiles_differ_outside_gates( let new_grants = new_profile.0.get(scope); if scope == "*" { - if without_gate_derived(old_grants, gated) != without_gate_derived(new_grants, gated) { + if without_gate_derived(old_grants, gated, allow_legacy_automatic_observe_removal) + != without_gate_derived(new_grants, gated, false) + { return true; } continue; @@ -137,7 +159,10 @@ fn management_differs_outside_gates( | ( ManagementPermissions::Override(old_profile), ManagementPermissions::Override(new_profile), - ) => profiles_differ_outside_gates(old_profile, new_profile, gated), + ) => profiles_differ_outside_gates(old_profile, new_profile, gated, true), + (ManagementPermissions::Extend(old_profile), ManagementPermissions::Auto) => { + profiles_differ_outside_gates(old_profile, &PermissionProfile::new(), gated, true) + } _ => true, } } @@ -172,7 +197,7 @@ impl StackCompatibilityCheck for PermissionProfilesUnchangedCheck { for (profile_name, new_profile) in new_profiles { if let Some(old_profile) = old_profiles.get(profile_name) { // Profile exists in both - check if it was modified - if profiles_differ_outside_gates(old_profile, new_profile, &gated) { + if profiles_differ_outside_gates(old_profile, new_profile, &gated, false) { errors.push(format!( "Permission profile '{}' was modified", profile_name @@ -460,6 +485,118 @@ mod tests { assert!(result.errors[0].contains("Management permissions")); } + #[tokio::test] + async fn a_legacy_automatic_observe_grant_can_leave_management() { + let old_stack = Stack::new("s".to_string()) + .management(ManagementPermissions::Extend( + PermissionProfile::new().global(["observe/observe", "worker/provision"]), + )) + .build(); + let new_stack = Stack::new("s".to_string()) + .management(ManagementPermissions::Extend( + PermissionProfile::new().global(["worker/provision"]), + )) + .build(); + + let result = PermissionProfilesUnchangedCheck + .check(&old_stack, &new_stack) + .await + .expect("check should run"); + + assert!(result.success, "{:?}", result.errors); + assert!(result.errors.is_empty()); + assert!(result.warnings.is_empty()); + } + + #[tokio::test] + async fn a_legacy_observe_only_management_profile_can_return_to_auto() { + let old_stack = Stack::new("s".to_string()) + .management(ManagementPermissions::Extend( + PermissionProfile::new().global(["observe/observe"]), + )) + .build(); + let new_stack = Stack::new("s".to_string()).build(); + + let result = PermissionProfilesUnchangedCheck + .check(&old_stack, &new_stack) + .await + .expect("check should run"); + + assert!(result.success, "{:?}", result.errors); + assert!(result.errors.is_empty()); + assert!(result.warnings.is_empty()); + } + + #[tokio::test] + async fn an_observe_grant_cannot_enter_management() { + let old_stack = Stack::new("s".to_string()) + .management(ManagementPermissions::Extend( + PermissionProfile::new().global(["worker/provision"]), + )) + .build(); + let new_stack = Stack::new("s".to_string()) + .management(ManagementPermissions::Extend( + PermissionProfile::new().global(["observe/observe", "worker/provision"]), + )) + .build(); + + let result = PermissionProfilesUnchangedCheck + .check(&old_stack, &new_stack) + .await + .expect("check should run"); + + assert!(!result.success); + assert_eq!( + result.errors, + vec!["Management permissions configuration was modified"] + ); + assert!(result.warnings.is_empty()); + } + + #[tokio::test] + async fn observe_removal_from_a_named_profile_is_still_rejected() { + let old_profile = PermissionProfile::new().global(["observe/observe", "worker/execute"]); + let new_profile = PermissionProfile::new().global(["worker/execute"]); + + let mut old_profiles = IndexMap::new(); + old_profiles.insert("application".to_string(), old_profile); + let mut new_profiles = IndexMap::new(); + new_profiles.insert("application".to_string(), new_profile); + + let old_stack = Stack { + id: "s".to_string(), + resources: IndexMap::new(), + permissions: PermissionsConfig { + profiles: old_profiles, + management: ManagementPermissions::Auto, + }, + supported_platforms: None, + inputs: vec![], + }; + let new_stack = Stack { + id: "s".to_string(), + resources: IndexMap::new(), + permissions: PermissionsConfig { + profiles: new_profiles, + management: ManagementPermissions::Auto, + }, + supported_platforms: None, + inputs: vec![], + }; + + let result = PermissionProfilesUnchangedCheck + .check(&old_stack, &new_stack) + .await + .expect("check should run"); + + assert!(!result.success); + assert_eq!( + result.errors, + vec!["Permission profile 'application' was modified"] + ); + assert!(result.warnings.is_empty()); + } + #[tokio::test] async fn test_unchanged_profiles_success() { let mut profile = PermissionProfile::new(); diff --git a/crates/alien-preflights/src/mutations/management_permission_profile.rs b/crates/alien-preflights/src/mutations/management_permission_profile.rs index 4e16ee829..853e1b8a7 100644 --- a/crates/alien-preflights/src/mutations/management_permission_profile.rs +++ b/crates/alien-preflights/src/mutations/management_permission_profile.rs @@ -12,7 +12,7 @@ use alien_permissions::get_permission_set; use indexmap::IndexMap; use std::collections::BTreeSet; -const OBSERVE_PERMISSION_SET_ID: &str = "observe/observe"; +pub(crate) const OBSERVE_PERMISSION_SET_ID: &str = "observe/observe"; /// Automatically adds management permission profile with necessary permissions for all resources in the stack. ///