Fix CspBuilder not applying fallback to uninitialized directive (#23855)

* Fix CspBuilder not applying fallback to uninitialized directive

* Improve test further

---------

Co-authored-by: Daniel Beck <daniel-beck@users.noreply.github.com>

Author: daniel-beck

core/src/main/java/jenkins/security/csp/CspBuilder.java

@@ -185,43 +185,37 @@ public CspBuilder initialize(FetchDirective fetchDirective, String... values) {
     public List<Directive> getMergedDirectives() {
         List<Directive> result = new ArrayList<>();
         for (Map.Entry<String, Set<String>> entry : directives.entrySet()) {
-            Set<String> effectiveValues = new HashSet<>();
             final String name = entry.getKey();
 
-            // Calculate inherited values from fallback chain
             if (FetchDirective.isFetchDirective(name)) {
+                // Calculate inherited values from fallback chain
                 FetchDirective current = FetchDirective.fromKey(name);
-
-                // Check if this directive was initialized
-                boolean wasInitialized = initializedFDs.contains(current);
-
-                // If NOT initialized, traverse fallback chain
-                if (!wasInitialized) {
-                    FetchDirective fallback = current.getFallback();
-                    while (fallback != null && !initializedFDs.contains(fallback)) {
-                        fallback = fallback.getFallback();
-                    }
-                    if (fallback == null) {
-                        // This could happen if nothing was initialized, in that case, fallback is default-src
-                        fallback = FetchDirective.DEFAULT_SRC;
-                    }
-                    // If we found an initialized fallback, inherit its values
-                    if (directives.containsKey(fallback.toKey())) {
-                        effectiveValues.addAll(directives.get(fallback.toKey()));
-                    }
-                }
-
-                effectiveValues.addAll(entry.getValue());
-                result.add(new Directive(name, !wasInitialized, List.copyOf(effectiveValues)));
+                result.add(new Directive(name, !initializedFDs.contains(current), List.copyOf(getEffectiveValues(current))));
             } else {
                 // Non-fetch directives don't inherit
-                effectiveValues.addAll(entry.getValue());
-                result.add(new Directive(name, null, List.copyOf(effectiveValues)));
+                result.add(new Directive(name, null, List.copyOf(entry.getValue())));
             }
         }
         return List.copyOf(result);
     }
 
+    private Set<String> getEffectiveValues(FetchDirective fetchDirective) {
+        if (fetchDirective == null) {
+            return Set.of();
+        }
+
+        final Set<String> currentDirectiveValues = directives.getOrDefault(fetchDirective.toKey(), Set.of());
+
+        if (initializedFDs.contains(fetchDirective)) {
+            return new HashSet<>(currentDirectiveValues);
+        }
+
+        Set<String> effectiveValues = new HashSet<>();
+        effectiveValues.addAll(getEffectiveValues(fetchDirective.getFallback()));
+        effectiveValues.addAll(currentDirectiveValues);
+        return effectiveValues;
+    }
+
     /**
      * Build the final CSP string. Any directives with no values left will have the 'none' value set.
      *

core/src/test/java/jenkins/security/csp/CspBuilderTest.java

@@ -429,6 +429,21 @@ void testNullValueAddition() {
         assertThat(builder.build(), is("img-src example.org;"));
     }
 
+    @Test
+    void testLongChain() {
+        CspBuilder builder = new CspBuilder();
+        builder.initialize(FetchDirective.DEFAULT_SRC, Directive.SELF);
+        builder.add(Directive.SCRIPT_SRC, "example.org");
+        builder.add(Directive.SCRIPT_SRC_ELEM, "example.com");
+        assertThat(builder.build(), is("default-src 'self'; script-src 'self' example.org; script-src-elem 'self' example.com example.org;"));
+
+        builder.initialize(FetchDirective.SCRIPT_SRC);
+        assertThat(builder.build(), is("default-src 'self'; script-src example.org; script-src-elem example.com example.org;"));
+
+        builder.initialize(FetchDirective.SCRIPT_SRC_ELEM);
+        assertThat(builder.build(), is("default-src 'self'; script-src example.org; script-src-elem example.com;"));
+    }
+
     private static Matcher<LogRecord> logMessageContainsString(String needle) {
         return new LogMessageContainsString(containsString(needle));
     }