Fix: Handle disabled or uninstalled permissions gracefully

pom.xml

@@ -1,12 +1,13 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
     <modelVersion>4.0.0</modelVersion>
 
     <parent>
         <groupId>org.jenkins-ci.plugins</groupId>
         <artifactId>plugin</artifactId>
         <version>5.3</version>
-        <relativePath />
+        <relativePath/>
     </parent>
 
     <groupId>io.jenkins.plugins</groupId>
@@ -23,6 +24,11 @@
             <name>Abhyudaya Sharma</name>
             <email>sharmaabhyudaya@gmail.com</email>
         </developer>
+        <developer>
+            <id>
+                csturiale
+            </id>
+        </developer>
     </developers>
 
     <properties>

src/main/java/io/jenkins/plugins/folderauth/FolderAuthorizationStrategyManagementLink.java

@@ -386,6 +386,8 @@ public void doDeleteAgentRole(@QueryParameter(required = true) String roleName)
     static Set<Permission> getSafePermissions(Set<PermissionGroup> groups) {
         TreeSet<Permission> safePermissions = new TreeSet<>(Permission.ID_COMPARATOR);
         groups.stream().map(PermissionGroup::getPermissions).forEach(safePermissions::addAll);
+        // SECURITY-3062: remove disabled permissions so they cannot be selected in the UI
+        safePermissions.removeIf(p -> !p.enabled);
         safePermissions.removeAll(PermissionWrapper.DANGEROUS_PERMISSIONS);
         return safePermissions;
     }

src/main/java/io/jenkins/plugins/folderauth/FolderBasedAuthorizationStrategy.java

@@ -81,9 +81,10 @@
     @DataBoundConstructor
     public FolderBasedAuthorizationStrategy(Set<GlobalRole> globalRoles, Set<FolderRole> folderRoles,
                                             Set<AgentRole> agentRoles) {
-        this.agentRoles = new HashSet<>(agentRoles);
-        this.globalRoles = new HashSet<>(globalRoles);
-        this.folderRoles = new HashSet<>(folderRoles);
+        // Guard against null — JCasC passes null when a parameter is omitted from YAML
+        this.agentRoles  = agentRoles  != null ? new HashSet<>(agentRoles)  : new HashSet<>();
+        this.globalRoles = globalRoles != null ? new HashSet<>(globalRoles) : new HashSet<>();
+        this.folderRoles = folderRoles != null ? new HashSet<>(folderRoles) : new HashSet<>();
 
         // the sets above should NOT be modified. They are not Collections.unmodifiableSet()
         // because that complicates the serialized XML and add unnecessary nesting.
@@ -270,7 +271,10 @@
             acl = new GenericAclImpl();
         }
         acl.assignPermissions(role.getSids(),
-            role.getPermissionsUnsorted().stream().map(PermissionWrapper::getPermission).collect(Collectors.toSet()));
+            role.getPermissionsUnsorted().stream()
+                .filter(PermissionWrapper::isValid)
+                .map(PermissionWrapper::getPermission)
+                .collect(Collectors.toSet()));
         acls.put(fullName, acl);
     }
 

src/main/java/io/jenkins/plugins/folderauth/acls/GlobalAclImpl.java

@@ -24,7 +24,10 @@ public GlobalAclImpl(Set<GlobalRole> globalRoles) {
         for (GlobalRole role : globalRoles) {
             Set<Permission> impliedPermissions = ConcurrentHashMap.newKeySet();
 
-            role.getPermissionsUnsorted().parallelStream().map(PermissionWrapper::getPermission).forEach(impliedPermissions::add);
+            role.getPermissionsUnsorted().parallelStream()
+                .filter(PermissionWrapper::isValid)
+                .map(PermissionWrapper::getPermission)
+                .forEach(impliedPermissions::add);
 
             role.getSids().parallelStream().forEach(sid -> {
                 Set<Permission> permissionsForSid = permissionList.get(sid);

src/main/java/io/jenkins/plugins/folderauth/misc/PermissionWrapper.java

@@ -10,6 +10,8 @@
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import javax.annotation.ParametersAreNonnullByDefault;
@@ -24,6 +26,7 @@
  */
 @ParametersAreNonnullByDefault
 public final class PermissionWrapper implements Comparable<PermissionWrapper> {
+    private static final Logger LOGGER = Logger.getLogger(PermissionWrapper.class.getName());
     // should've been final but needs to be setup when the
     // object is deserialized from the XML config
     private transient Permission permission;
@@ -49,6 +52,10 @@
     }
 
     public String getId() {
+        // If the permission could not be resolved (unknown plugin/disabled), fall back to the raw stored id.
+        if (permission == null) {
+            return id;
+        }
         return String.format("%s/%s", permission.group.getId(), permission.name);
     }
 
@@ -66,15 +73,22 @@
     }
 
     /**
-     * Get the permission corresponding to this {@link PermissionWrapper}
-     *
-     * @return the permission corresponding to this {@link PermissionWrapper}
+     * Get the permission corresponding to this {@link PermissionWrapper}.
+     * May return {@code null} when the permission ID references a plugin that is not installed
+     * or a permission that could not be resolved.  Callers must handle the {@code null} case.
      */
-    @NonNull
     public Permission getPermission() {
         return permission;
     }
 
+    /**
+     * Returns {@code true} if this wrapper holds a valid, enabled permission.
+     * Wrappers for which this returns {@code false} should not be used for access-control decisions.
+     */
+    public boolean isValid() {
+        return permission != null && permission.enabled;
+    }
+
 
     @Override
     public boolean equals(Object o) {
@@ -92,13 +106,36 @@
     /**
      * Checks if the permission for this {@link PermissionWrapper} is valid.
      *
-     * @throws IllegalArgumentException when the permission did not exist, was null or was dangerous.
+     * <p>Behaviour by case:
+     * <ul>
+     *   <li><b>Dangerous permissions</b> ({@link #DANGEROUS_PERMISSIONS}) – always rejected with an
+     *       {@link IllegalArgumentException} regardless of context.</li>
+     *   <li><b>Unknown permissions</b> ({@code permission == null}) – a WARNING is logged and the
+     *       wrapper is kept with a {@code null} internal permission.  This allows Jenkins to start up
+     *       even when a referenced permission belongs to a plugin that is not currently installed.</li>
+     *   <li><b>Disabled permissions</b> ({@code !permission.enabled}) – a WARNING is logged.
+     *       SECURITY-3062 compliance is preserved via two mechanisms: the UI filters disabled
+     *       permissions out ({@code getSafePermissions}) and Jenkins core refuses to honour them
+     *       at access-check time ({@code AbstractACL.hasPermission}).</li>
+     * </ul>
      */
     private void checkPermission() {
         if (permission == null) {
-            throw new IllegalArgumentException(Messages.PermissionWrapper_UnknownPermission(id));
+            // Permission from an uninstalled/unavailable plugin – log and continue rather than crashing.
+            LOGGER.log(Level.WARNING,
+                    "Permission ''{0}'' is unknown in this Jenkins installation (plugin may not be installed) "
+                    + "and will have no effect.", id);
         } else if (DANGEROUS_PERMISSIONS.contains(permission)) {
             throw new IllegalArgumentException(Messages.PermissionWrapper_NoDangerousPermissions());
+        } else if (!permission.enabled) {
+            // SECURITY-3062: disabled permissions cannot be granted via the UI (getSafePermissions filters
+            // them out) and Jenkins core itself refuses to honour them at access-check time.  We log a
+            // WARNING here instead of throwing so that existing configurations that reference a permission
+            // which is disabled in the current environment (e.g. Credentials/UseItem, Job/WipeOut) do not
+            // prevent Jenkins from starting up.
+            LOGGER.log(Level.WARNING,
+                    "Permission ''{0}'' is disabled in this Jenkins installation and will have no effect. "
+                    + "Consider removing it from the folder-auth configuration.", id);
         }
     }
 
@@ -134,6 +171,10 @@
 
     @Override
     public int compareTo(@NonNull PermissionWrapper other) {
+        // Fall back to string comparison when either permission could not be resolved.
+        if (this.permission == null || other.permission == null) {
+            return this.id.compareTo(other.id);
+        }
         return Permission.ID_COMPARATOR.compare(this.permission, other.permission);
     }
 }

src/test/java/io/jenkins/plugins/folderauth/misc/PermissionWrapperTest.java

@@ -5,6 +5,9 @@
 import org.junit.Test;
 import org.jvnet.hudson.test.JenkinsRule;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+
 public class PermissionWrapperTest {
     @ClassRule
     public static JenkinsRule j = new JenkinsRule();
@@ -14,8 +17,15 @@ public void shouldNotAllowDangerousPermissions() {
         new PermissionWrapper(Jenkins.RUN_SCRIPTS.getId());
     }
 
-    @Test(expected = IllegalArgumentException.class)
-    public void shouldNotAllowNullPermissions() {
-        new PermissionWrapper("this is not a permission id");
+    /**
+     * Unknown permission IDs (e.g. from an uninstalled plugin) no longer throw.
+     * Instead the wrapper is created with a null internal permission and {@link PermissionWrapper#isValid()}
+     * returns {@code false}.  Jenkins will not grant any access for such wrappers.
+     */
+    @Test
+    public void unknownPermissionsAreAcceptedWithWarning() {
+        PermissionWrapper wrapper = new PermissionWrapper("this is not a permission id");
+        assertNull("permission should be null for an unknown id", wrapper.getPermission());
+        assertFalse("wrapper with unknown permission must not be valid", wrapper.isValid());
     }
 }