From 13f835ccdf7924c7b8c3484a563b94aa0c97468e Mon Sep 17 00:00:00 2001 From: Aitor Lopez Date: Thu, 30 Jul 2026 11:53:38 +0200 Subject: [PATCH 1/6] Propagate readOnlyMode into SCM Checkout Strategy descriptor config f:dropdownDescriptorSelector renders its selected descriptor's config.jelly lazily via AJAX (lib/form/dropdownDescriptorSelector.jelly), and only variables listed in its `capture` attribute are visible to that lazily rendered fragment. hudson/model/Job/configure.jelly sets readOnlyMode for the whole config page, but lib/hudson/project/config-scm.jelly's call for scmCheckoutStrategy didn't capture it, so a Job's SCM Checkout Strategy config fragment was not disabled for read-only viewers (Item.READ + Item.EXTENDED_READ, no Item.CONFIGURE), even though every other field on the page correctly disables in that case. This mirrors the same gap that was originally found and fixed in cloudbees-folder-plugin (JENKINS-62218, jenkinsci/cloudbees-folder-plugin#747) and reverted there since it's a core issue affecting every Job configuration page, not just folders. Part of the JENKINS-12548 read-only system configuration browsing epic (JEP-224). No Jira ticket filed yet for this specific core gap. --- core/src/main/resources/lib/hudson/project/config-scm.jelly | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/resources/lib/hudson/project/config-scm.jelly b/core/src/main/resources/lib/hudson/project/config-scm.jelly index 56c174b11887..331cb31d8002 100644 --- a/core/src/main/resources/lib/hudson/project/config-scm.jelly +++ b/core/src/main/resources/lib/hudson/project/config-scm.jelly @@ -44,7 +44,7 @@ THE SOFTWARE. + descriptors="${descriptors}" capture="readOnlyMode" /> From 4c6ce0fdbc1adb113bfc8366e7ef85ce79ae282a Mon Sep 17 00:00:00 2001 From: Aitor Lopez Date: Thu, 30 Jul 2026 13:34:05 +0200 Subject: [PATCH 2/6] Propagate readOnlyMode to remaining dropdownDescriptorSelector fragments f:dropdownDescriptorSelector defers rendering of every non-selected descriptor's config.jelly/config.groovy via l:renderOnDemand, which only forwards variables named in its `capture` attribute into that later AJAX-rendered fragment. readOnlyMode (JEP-224, JENKINS-12548) is not forwarded by default, so a viewer with Item.EXTENDED_READ but not Item.CONFIGURE can get back a fully editable fragment for any option other than the currently selected one, even though the rest of the page correctly disables. The gap was first found and fixed for the SCM Checkout Strategy dropdown on Job/configure.jelly (previous commit, also reported against cloudbees-folder-plugin#747 / JENKINS-62218 and reverted there as a core issue). Review of that fix pointed out the bug is generic to dropdownDescriptorSelector, not specific to SCM Checkout Strategy. This commit closes the remaining Job/agent-configuration-scoped call sites found by auditing all *.jelly and *.groovy usages of the tag in core: - HudsonPrivateSecurityRealm/config.jelly (Password Complexity Rule) - DelegatingComputerLauncher/config.jelly and DumbSlave/configure-entries.jelly (agent Launch method, Availability/retention strategy) - Maven/config.jelly (Settings file, Global Settings file build-step config) - GlobalFingerprintConfiguration/config.jelly (Fingerprint Storage Engine) - BuildDiscarderProperty/config-details.jelly and SimpleGlobalBuildDiscarderStrategy/config.jelly (Discard old builds strategy, at both the per-job and global-default nesting levels) - GlobalMavenConfig/config.groovy (default settings/global settings provider) - ViewsTabBar/GlobalConfigurationImpl/config.groovy (Views Tab Bar) - GlobalCrumbIssuerConfiguration/config.groovy (Crumb Issuer) hudson/model/MyViewsProperty/config.jelly (Views Tab Bar on a user's own account page) was deliberately left out: that page requires full Jenkins.ADMINISTER and never sets readOnlyMode in its render chain, so it is not an instance of this bug. A further instance of this same gap exists on the "Manage Jenkins > Security" page (Security Realm / Authorization Strategy / Markup Formatter selectors), reachable with only Overall.SYSTEM_READ. That one is being reported through the Jenkins Security Team's private disclosure process instead of here, since it can expose an editable Security Realm/Authorization Strategy fragment to a low-privilege viewer -- a permission-escalation-adjacent issue rather than a plain read-only-UI gap. Adds a regression test (SCMCheckoutStrategyReadOnlyModeTest) that registers a throwaway SCMCheckoutStrategy with a real editable field, triggers the same renderOnDemand() JS a browser fires when switching the dropdown, and asserts the resulting fragment is rendered as the standard read-only N/A placeholder rather than an editable input. --- .../HudsonPrivateSecurityRealm/config.jelly | 2 +- .../config.groovy | 3 +- .../DelegatingComputerLauncher/config.jelly | 2 +- .../slaves/DumbSlave/configure-entries.jelly | 4 +- .../resources/hudson/tasks/Maven/config.jelly | 4 +- .../GlobalConfigurationImpl/config.groovy | 2 +- .../config.jelly | 2 +- .../config-details.jelly | 2 +- .../config.jelly | 2 +- .../mvn/GlobalMavenConfig/config.groovy | 4 +- .../SCMCheckoutStrategyReadOnlyModeTest.java | 95 +++++++++++++++++++ .../FieldSCMCheckoutStrategy/config.jelly | 7 ++ 12 files changed, 116 insertions(+), 13 deletions(-) create mode 100644 test/src/test/java/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest.java create mode 100644 test/src/test/resources/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest/FieldSCMCheckoutStrategy/config.jelly diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/config.jelly b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/config.jelly index f638291ee6b7..917dd7a1a0d7 100644 --- a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/config.jelly +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/config.jelly @@ -27,7 +27,7 @@ THE SOFTWARE. - + - + diff --git a/core/src/main/resources/hudson/slaves/DumbSlave/configure-entries.jelly b/core/src/main/resources/hudson/slaves/DumbSlave/configure-entries.jelly index e70e455be703..9056a295126c 100644 --- a/core/src/main/resources/hudson/slaves/DumbSlave/configure-entries.jelly +++ b/core/src/main/resources/hudson/slaves/DumbSlave/configure-entries.jelly @@ -50,12 +50,12 @@ THE SOFTWARE. - + - + - - + + diff --git a/core/src/main/resources/hudson/views/ViewsTabBar/GlobalConfigurationImpl/config.groovy b/core/src/main/resources/hudson/views/ViewsTabBar/GlobalConfigurationImpl/config.groovy index 96578b4b9281..127e51faac34 100644 --- a/core/src/main/resources/hudson/views/ViewsTabBar/GlobalConfigurationImpl/config.groovy +++ b/core/src/main/resources/hudson/views/ViewsTabBar/GlobalConfigurationImpl/config.groovy @@ -6,5 +6,5 @@ def f=namespace(lib.FormTagLib) def all = ViewsTabBar.all() if (all.size()>1) { - f.dropdownDescriptorSelector(title:_("Views Tab Bar"),field:"viewsTabBar") + f.dropdownDescriptorSelector(title:_("Views Tab Bar"),field:"viewsTabBar",capture:"readOnlyMode") } diff --git a/core/src/main/resources/jenkins/fingerprints/GlobalFingerprintConfiguration/config.jelly b/core/src/main/resources/jenkins/fingerprints/GlobalFingerprintConfiguration/config.jelly index 406adcc7a3eb..b547df2bfd79 100644 --- a/core/src/main/resources/jenkins/fingerprints/GlobalFingerprintConfiguration/config.jelly +++ b/core/src/main/resources/jenkins/fingerprints/GlobalFingerprintConfiguration/config.jelly @@ -26,7 +26,7 @@ THE SOFTWARE. - + diff --git a/core/src/main/resources/jenkins/model/BuildDiscarderProperty/config-details.jelly b/core/src/main/resources/jenkins/model/BuildDiscarderProperty/config-details.jelly index a5f3d0a4d713..16a6184df32b 100644 --- a/core/src/main/resources/jenkins/model/BuildDiscarderProperty/config-details.jelly +++ b/core/src/main/resources/jenkins/model/BuildDiscarderProperty/config-details.jelly @@ -25,5 +25,5 @@ THE SOFTWARE. - + diff --git a/core/src/main/resources/jenkins/model/SimpleGlobalBuildDiscarderStrategy/config.jelly b/core/src/main/resources/jenkins/model/SimpleGlobalBuildDiscarderStrategy/config.jelly index 8e9f880f9a6f..df6cf9c3e847 100644 --- a/core/src/main/resources/jenkins/model/SimpleGlobalBuildDiscarderStrategy/config.jelly +++ b/core/src/main/resources/jenkins/model/SimpleGlobalBuildDiscarderStrategy/config.jelly @@ -3,5 +3,5 @@ ${%blurb} - + diff --git a/core/src/main/resources/jenkins/mvn/GlobalMavenConfig/config.groovy b/core/src/main/resources/jenkins/mvn/GlobalMavenConfig/config.groovy index 7d1ff84aa72f..e0e80978aebd 100644 --- a/core/src/main/resources/jenkins/mvn/GlobalMavenConfig/config.groovy +++ b/core/src/main/resources/jenkins/mvn/GlobalMavenConfig/config.groovy @@ -3,6 +3,6 @@ package jenkins.mvn.GlobalMavenConfig def f = namespace(lib.FormTagLib) f.section(title:_("Maven Configuration")) { - f.dropdownDescriptorSelector(title:_("Default settings provider"), field:"settingsProvider") - f.dropdownDescriptorSelector(title:_("Default global settings provider"), field:"globalSettingsProvider") + f.dropdownDescriptorSelector(title:_("Default settings provider"), field:"settingsProvider", capture:"readOnlyMode") + f.dropdownDescriptorSelector(title:_("Default global settings provider"), field:"globalSettingsProvider", capture:"readOnlyMode") } diff --git a/test/src/test/java/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest.java b/test/src/test/java/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest.java new file mode 100644 index 000000000000..555a349ce326 --- /dev/null +++ b/test/src/test/java/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest.java @@ -0,0 +1,95 @@ +package jenkins.scm; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import hudson.model.AbstractProject; +import hudson.model.FreeStyleProject; +import hudson.model.Item; +import jenkins.model.Jenkins; +import org.htmlunit.WebClientUtil; +import org.htmlunit.html.DomElement; +import org.htmlunit.html.HtmlPage; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.MockAuthorizationStrategy; +import org.jvnet.hudson.test.TestExtension; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; +import org.kohsuke.stapler.DataBoundConstructor; + +/** + * Reproduces JENKINS-12548: the SCM Checkout Strategy dropdown in + * {@code config-scm.jelly} renders non-selected descriptors lazily via + * {@code l:renderOnDemand}, which only forwards variables named in its + * {@code capture} attribute. Without {@code capture="readOnlyMode"}, a + * viewer with {@link Item#EXTENDED_READ} but not {@link Item#CONFIGURE} + * gets an editable field once that fragment is loaded. + */ +@WithJenkins +class SCMCheckoutStrategyReadOnlyModeTest { + + private JenkinsRule j; + + @BeforeEach + void setUp(JenkinsRule rule) { + j = rule; + } + + @Issue("JENKINS-12548") + @Test + void lazilyLoadedStrategyFragmentIsDisabledForReadOnlyViewer() throws Exception { + FreeStyleProject p = j.createFreeStyleProject("p"); + // Leave the default strategy selected so FieldSCMCheckoutStrategy's + // config.jelly is the one deferred via l:renderOnDemand. + + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() + .grant(Jenkins.ADMINISTER).everywhere().to("admin") + .grant(Jenkins.READ, Item.READ, Item.EXTENDED_READ).everywhere().to("viewer")); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.withBasicCredentials("viewer"); + HtmlPage page = wc.goTo("job/" + p.getName() + "/configure"); + + // Triggers the same client-side fetch normally fired when the user picks + // a non-default option from the "SCM Checkout Strategy" dropdown. + page.executeJavaScript( + "document.getElementsBySelector('.render-on-demand').forEach(function(e) { renderOnDemand(e); })"); + WebClientUtil.waitForJSExec(wc); + + DomElement strategyBlock = page.querySelector("div[name='scmCheckoutStrategy']"); + assertNotNull(strategyBlock, "expected the SCM Checkout Strategy dropdown block to be rendered"); + + assertNull(strategyBlock.querySelector("input[name='_.value']"), + "the lazily-loaded FieldSCMCheckoutStrategy fragment must not expose an editable 'value' field" + + " to a read-only (Item.EXTENDED_READ, no Item.CONFIGURE) viewer"); + assertNotNull(strategyBlock.querySelector(".jenkins-not-applicable"), + "the read-only viewer should instead see the field rendered as the standard N/A read-only placeholder"); + } + + @SuppressWarnings("rawtypes") + public static class FieldSCMCheckoutStrategy extends SCMCheckoutStrategy { + + private final String value; + + @SuppressWarnings("checkstyle:redundantmodifier") + @DataBoundConstructor + public FieldSCMCheckoutStrategy(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + + @TestExtension + public static class DescriptorImpl extends SCMCheckoutStrategyDescriptor { + @Override + public boolean isApplicable(AbstractProject project) { + return true; + } + } + } +} diff --git a/test/src/test/resources/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest/FieldSCMCheckoutStrategy/config.jelly b/test/src/test/resources/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest/FieldSCMCheckoutStrategy/config.jelly new file mode 100644 index 000000000000..69a00c9b4117 --- /dev/null +++ b/test/src/test/resources/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest/FieldSCMCheckoutStrategy/config.jelly @@ -0,0 +1,7 @@ + + + + + + + From dda9cfa11069a7daf2f8db301062dd559d15785d Mon Sep 17 00:00:00 2001 From: Aitor Lopez Date: Thu, 30 Jul 2026 14:05:21 +0200 Subject: [PATCH 3/6] Propagate readOnlyMode to GlobalSecurityConfiguration's dropdownDescriptorSelector calls Applies the same fix as the previous commit to the last remaining dropdownDescriptorSelector call sites in core: Security Realm, Authorization, and Markup Formatter on hudson/security/GlobalSecurityConfiguration/index.groovy ("Manage Jenkins > Security", reachable with only Overall.SYSTEM_READ). This was initially held back to verify it wasn't a privilege-escalation risk before publishing: GlobalSecurityConfiguration#configure() unconditionally calls j.checkPermission(Jenkins.ADMINISTER) as the first line, before any submitted JSON is read, so a forged configSubmit POST from a SYSTEM_READ-only user is rejected server-side (AccessDeniedException3) regardless of what the UI rendered; the Save/Apply buttons are also only rendered inside l.isAdmin(). The lazily-rendered fragment itself never carries live secrets either, since dropdownDescriptorSelector renders non-selected options with instance=null. Confirmed the same double gate (checkPermission(CONFIGURE) in doConfigSubmit) exists for the other already-fixed call sites, so this is the same severity class as the rest of the change: a read-only-mode UI/UX gap, not a write-path vulnerability. Adds a regression test (GlobalSecurityConfigurationReadOnlyModeTest) covering this Groovy-view, SYSTEM_READ-scoped rendering path, following the same pattern as SCMCheckoutStrategyReadOnlyModeTest: register a throwaway AuthorizationStrategy with a real editable field, trigger the same renderOnDemand() JS a browser fires when switching the dropdown, and assert the fragment renders as the standard read-only N/A placeholder. --- .../GlobalSecurityConfiguration/index.groovy | 6 +- ...SecurityConfigurationReadOnlyModeTest.java | 106 ++++++++++++++++++ .../FieldAuthorizationStrategy/config.jelly | 7 ++ 3 files changed, 116 insertions(+), 3 deletions(-) create mode 100644 test/src/test/java/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest.java create mode 100644 test/src/test/resources/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest/FieldAuthorizationStrategy/config.jelly diff --git a/core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy b/core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy index c9555cf750b4..a439c9f72470 100644 --- a/core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy +++ b/core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy @@ -22,12 +22,12 @@ l.'settings-subpage'(permission: app.SYSTEM_READ) { f.entry(help: '/descriptor/hudson.security.GlobalSecurityConfiguration/help/disableRememberMe') { f.checkbox(title:_("Disable remember me"), field: "disableRememberMe") } - f.dropdownDescriptorSelector(title: _("Security Realm"), field: 'securityRealm', descriptors: h.filterDescriptors(app, SecurityRealm.all())) - f.dropdownDescriptorSelector(title: _("Authorization"), field: 'authorizationStrategy', descriptors: h.filterDescriptors(app, AuthorizationStrategy.all())) + f.dropdownDescriptorSelector(title: _("Security Realm"), field: 'securityRealm', descriptors: h.filterDescriptors(app, SecurityRealm.all()), capture: "readOnlyMode") + f.dropdownDescriptorSelector(title: _("Authorization"), field: 'authorizationStrategy', descriptors: h.filterDescriptors(app, AuthorizationStrategy.all()), capture: "readOnlyMode") } f.section(title: _("Markup Formatter")) { - f.dropdownDescriptorSelector(title:_("Markup Formatter"), descriptors: MarkupFormatterDescriptor.all(), field: 'markupFormatter') + f.dropdownDescriptorSelector(title:_("Markup Formatter"), descriptors: MarkupFormatterDescriptor.all(), field: 'markupFormatter', capture: "readOnlyMode") } f.section(title: _("Agents")) { diff --git a/test/src/test/java/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest.java b/test/src/test/java/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest.java new file mode 100644 index 000000000000..f97032657088 --- /dev/null +++ b/test/src/test/java/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest.java @@ -0,0 +1,106 @@ +package hudson.security; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import edu.umd.cs.findbugs.annotations.NonNull; +import hudson.model.Descriptor; +import java.util.Collection; +import java.util.Collections; +import jenkins.model.Jenkins; +import org.htmlunit.WebClientUtil; +import org.htmlunit.html.DomElement; +import org.htmlunit.html.HtmlPage; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.MockAuthorizationStrategy; +import org.jvnet.hudson.test.TestExtension; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; +import org.kohsuke.stapler.DataBoundConstructor; + +/** + * Reproduces JENKINS-12548 on the "Manage Jenkins > Security" page + * ({@code hudson/security/GlobalSecurityConfiguration/index.groovy}), a + * second, architecturally distinct rendering path (Groovy view, reachable + * with only {@link Jenkins#SYSTEM_READ}) for the same underlying bug as + * {@link jenkins.scm.SCMCheckoutStrategyReadOnlyModeTest}: the "Authorization" + * {@code f.dropdownDescriptorSelector} lazily loads non-selected + * {@link AuthorizationStrategy} fragments via {@code l:renderOnDemand}, which + * only forwards variables named in its {@code capture} attribute. + */ +@WithJenkins +class GlobalSecurityConfigurationReadOnlyModeTest { + + private JenkinsRule j; + + @BeforeEach + void setUp(JenkinsRule rule) { + j = rule; + } + + @Issue("JENKINS-12548") + @Test + void lazilyLoadedAuthorizationStrategyFragmentIsDisabledForSystemReadViewer() throws Exception { + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() + .grant(Jenkins.ADMINISTER).everywhere().to("admin") + .grant(Jenkins.READ, Jenkins.SYSTEM_READ).everywhere().to("viewer")); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.withBasicCredentials("viewer"); + HtmlPage page = wc.goTo("configureSecurity"); + + // Triggers the same client-side fetch normally fired when the user picks + // a non-default option from the "Authorization" dropdown. + page.executeJavaScript( + "document.getElementsBySelector('.render-on-demand').forEach(function(e) { renderOnDemand(e); })"); + WebClientUtil.waitForJSExec(wc); + + DomElement authorizationBlock = page.querySelector("div[name='authorizationStrategy']"); + assertNotNull(authorizationBlock, "expected the Authorization dropdown block to be rendered"); + + assertNull(authorizationBlock.querySelector("input[name='_.value']"), + "the lazily-loaded FieldAuthorizationStrategy fragment must not expose an editable 'value' field" + + " to a viewer with only Jenkins.SYSTEM_READ (no Jenkins.ADMINISTER)"); + assertNotNull(authorizationBlock.querySelector(".jenkins-not-applicable"), + "the read-only viewer should instead see the field rendered as the standard N/A read-only placeholder"); + } + + public static class FieldAuthorizationStrategy extends AuthorizationStrategy { + + private final String value; + + @SuppressWarnings("checkstyle:redundantmodifier") + @DataBoundConstructor + public FieldAuthorizationStrategy(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + + @NonNull + @Override + public ACL getRootACL() { + return ACL.lambda2((a, p) -> false); + } + + @NonNull + @Override + public Collection getGroups() { + return Collections.emptySet(); + } + + @TestExtension + public static class DescriptorImpl extends Descriptor { + @NonNull + @Override + public String getDisplayName() { + return "FieldAuthorizationStrategy"; + } + } + } +} diff --git a/test/src/test/resources/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest/FieldAuthorizationStrategy/config.jelly b/test/src/test/resources/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest/FieldAuthorizationStrategy/config.jelly new file mode 100644 index 000000000000..69a00c9b4117 --- /dev/null +++ b/test/src/test/resources/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest/FieldAuthorizationStrategy/config.jelly @@ -0,0 +1,7 @@ + + + + + + + From 061162e9f012e8e75cc444a07968f076c8a8db32 Mon Sep 17 00:00:00 2001 From: Aitor Lopez Date: Mon, 3 Aug 2026 12:14:58 +0200 Subject: [PATCH 4/6] Propagate readOnlyMode once in dropdownDescriptorSelector taglib, not per caller Per jtnord's review on #27167: rather than requiring every caller of f:dropdownDescriptorSelector to pass capture="readOnlyMode" (12 call sites in core alone, plus any plugin using the same taglib), fix it once in the taglib itself. lib/form/dropdownDescriptorSelector.jelly already always captures "descriptor" and "it" for the lazily rendered l:renderOnDemand fragment regardless of what the caller's capture attribute lists. readOnlyMode now joins that same always-captured set, so read-only mode propagates into every lazily loaded descriptor fragment ecosystem-wide without any caller changes. This reverts the 12 per-callsite capture="readOnlyMode" additions from the previous commits on this branch and updates the two JenkinsRule/HtmlUnit regression tests' documentation to point at the new, single fix location. Both tests still pass unchanged, since they exercise the real page end-to-end rather than depending on any caller's capture attribute. --- .../security/GlobalSecurityConfiguration/index.groovy | 6 +++--- .../security/HudsonPrivateSecurityRealm/config.jelly | 2 +- .../csrf/GlobalCrumbIssuerConfiguration/config.groovy | 3 +-- .../slaves/DelegatingComputerLauncher/config.jelly | 2 +- .../hudson/slaves/DumbSlave/configure-entries.jelly | 4 ++-- core/src/main/resources/hudson/tasks/Maven/config.jelly | 4 ++-- .../ViewsTabBar/GlobalConfigurationImpl/config.groovy | 2 +- .../GlobalFingerprintConfiguration/config.jelly | 2 +- .../model/BuildDiscarderProperty/config-details.jelly | 2 +- .../SimpleGlobalBuildDiscarderStrategy/config.jelly | 2 +- .../jenkins/mvn/GlobalMavenConfig/config.groovy | 4 ++-- .../resources/lib/form/dropdownDescriptorSelector.jelly | 5 ++++- .../main/resources/lib/hudson/project/config-scm.jelly | 2 +- .../GlobalSecurityConfigurationReadOnlyModeTest.java | 4 +++- .../jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest.java | 9 ++++++--- 15 files changed, 30 insertions(+), 23 deletions(-) diff --git a/core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy b/core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy index a439c9f72470..c9555cf750b4 100644 --- a/core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy +++ b/core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy @@ -22,12 +22,12 @@ l.'settings-subpage'(permission: app.SYSTEM_READ) { f.entry(help: '/descriptor/hudson.security.GlobalSecurityConfiguration/help/disableRememberMe') { f.checkbox(title:_("Disable remember me"), field: "disableRememberMe") } - f.dropdownDescriptorSelector(title: _("Security Realm"), field: 'securityRealm', descriptors: h.filterDescriptors(app, SecurityRealm.all()), capture: "readOnlyMode") - f.dropdownDescriptorSelector(title: _("Authorization"), field: 'authorizationStrategy', descriptors: h.filterDescriptors(app, AuthorizationStrategy.all()), capture: "readOnlyMode") + f.dropdownDescriptorSelector(title: _("Security Realm"), field: 'securityRealm', descriptors: h.filterDescriptors(app, SecurityRealm.all())) + f.dropdownDescriptorSelector(title: _("Authorization"), field: 'authorizationStrategy', descriptors: h.filterDescriptors(app, AuthorizationStrategy.all())) } f.section(title: _("Markup Formatter")) { - f.dropdownDescriptorSelector(title:_("Markup Formatter"), descriptors: MarkupFormatterDescriptor.all(), field: 'markupFormatter', capture: "readOnlyMode") + f.dropdownDescriptorSelector(title:_("Markup Formatter"), descriptors: MarkupFormatterDescriptor.all(), field: 'markupFormatter') } f.section(title: _("Agents")) { diff --git a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/config.jelly b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/config.jelly index 917dd7a1a0d7..f638291ee6b7 100644 --- a/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/config.jelly +++ b/core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/config.jelly @@ -27,7 +27,7 @@ THE SOFTWARE. - + - + diff --git a/core/src/main/resources/hudson/slaves/DumbSlave/configure-entries.jelly b/core/src/main/resources/hudson/slaves/DumbSlave/configure-entries.jelly index 9056a295126c..e70e455be703 100644 --- a/core/src/main/resources/hudson/slaves/DumbSlave/configure-entries.jelly +++ b/core/src/main/resources/hudson/slaves/DumbSlave/configure-entries.jelly @@ -50,12 +50,12 @@ THE SOFTWARE. - + - + - - + + diff --git a/core/src/main/resources/hudson/views/ViewsTabBar/GlobalConfigurationImpl/config.groovy b/core/src/main/resources/hudson/views/ViewsTabBar/GlobalConfigurationImpl/config.groovy index 127e51faac34..96578b4b9281 100644 --- a/core/src/main/resources/hudson/views/ViewsTabBar/GlobalConfigurationImpl/config.groovy +++ b/core/src/main/resources/hudson/views/ViewsTabBar/GlobalConfigurationImpl/config.groovy @@ -6,5 +6,5 @@ def f=namespace(lib.FormTagLib) def all = ViewsTabBar.all() if (all.size()>1) { - f.dropdownDescriptorSelector(title:_("Views Tab Bar"),field:"viewsTabBar",capture:"readOnlyMode") + f.dropdownDescriptorSelector(title:_("Views Tab Bar"),field:"viewsTabBar") } diff --git a/core/src/main/resources/jenkins/fingerprints/GlobalFingerprintConfiguration/config.jelly b/core/src/main/resources/jenkins/fingerprints/GlobalFingerprintConfiguration/config.jelly index b547df2bfd79..406adcc7a3eb 100644 --- a/core/src/main/resources/jenkins/fingerprints/GlobalFingerprintConfiguration/config.jelly +++ b/core/src/main/resources/jenkins/fingerprints/GlobalFingerprintConfiguration/config.jelly @@ -26,7 +26,7 @@ THE SOFTWARE. - + diff --git a/core/src/main/resources/jenkins/model/BuildDiscarderProperty/config-details.jelly b/core/src/main/resources/jenkins/model/BuildDiscarderProperty/config-details.jelly index 16a6184df32b..a5f3d0a4d713 100644 --- a/core/src/main/resources/jenkins/model/BuildDiscarderProperty/config-details.jelly +++ b/core/src/main/resources/jenkins/model/BuildDiscarderProperty/config-details.jelly @@ -25,5 +25,5 @@ THE SOFTWARE. - + diff --git a/core/src/main/resources/jenkins/model/SimpleGlobalBuildDiscarderStrategy/config.jelly b/core/src/main/resources/jenkins/model/SimpleGlobalBuildDiscarderStrategy/config.jelly index df6cf9c3e847..8e9f880f9a6f 100644 --- a/core/src/main/resources/jenkins/model/SimpleGlobalBuildDiscarderStrategy/config.jelly +++ b/core/src/main/resources/jenkins/model/SimpleGlobalBuildDiscarderStrategy/config.jelly @@ -3,5 +3,5 @@ ${%blurb} - + diff --git a/core/src/main/resources/jenkins/mvn/GlobalMavenConfig/config.groovy b/core/src/main/resources/jenkins/mvn/GlobalMavenConfig/config.groovy index e0e80978aebd..7d1ff84aa72f 100644 --- a/core/src/main/resources/jenkins/mvn/GlobalMavenConfig/config.groovy +++ b/core/src/main/resources/jenkins/mvn/GlobalMavenConfig/config.groovy @@ -3,6 +3,6 @@ package jenkins.mvn.GlobalMavenConfig def f = namespace(lib.FormTagLib) f.section(title:_("Maven Configuration")) { - f.dropdownDescriptorSelector(title:_("Default settings provider"), field:"settingsProvider", capture:"readOnlyMode") - f.dropdownDescriptorSelector(title:_("Default global settings provider"), field:"globalSettingsProvider", capture:"readOnlyMode") + f.dropdownDescriptorSelector(title:_("Default settings provider"), field:"settingsProvider") + f.dropdownDescriptorSelector(title:_("Default global settings provider"), field:"globalSettingsProvider") } diff --git a/core/src/main/resources/lib/form/dropdownDescriptorSelector.jelly b/core/src/main/resources/lib/form/dropdownDescriptorSelector.jelly index 084e47717153..359c12d982b6 100644 --- a/core/src/main/resources/lib/form/dropdownDescriptorSelector.jelly +++ b/core/src/main/resources/lib/form/dropdownDescriptorSelector.jelly @@ -47,6 +47,9 @@ THE SOFTWARE. Config fragments from descriptors are rendered lazily by default, which means variables seen in the caller aren't visible to them. This attribute allows you to nominate additional variables and their values to be captured for descriptors. + Note that "readOnlyMode" is always captured automatically, so callers do not + need to (and should not need to) list it explicitly for read-only mode to be + propagated into the lazily rendered descriptor fragment. @@ -61,7 +64,7 @@ THE SOFTWARE. + lazy="descriptor,it,readOnlyMode,${capture}"> diff --git a/core/src/main/resources/lib/hudson/project/config-scm.jelly b/core/src/main/resources/lib/hudson/project/config-scm.jelly index 331cb31d8002..56c174b11887 100644 --- a/core/src/main/resources/lib/hudson/project/config-scm.jelly +++ b/core/src/main/resources/lib/hudson/project/config-scm.jelly @@ -44,7 +44,7 @@ THE SOFTWARE. + descriptors="${descriptors}" /> diff --git a/test/src/test/java/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest.java b/test/src/test/java/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest.java index f97032657088..c719c3649579 100644 --- a/test/src/test/java/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest.java +++ b/test/src/test/java/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest.java @@ -28,7 +28,9 @@ * {@link jenkins.scm.SCMCheckoutStrategyReadOnlyModeTest}: the "Authorization" * {@code f.dropdownDescriptorSelector} lazily loads non-selected * {@link AuthorizationStrategy} fragments via {@code l:renderOnDemand}, which - * only forwards variables named in its {@code capture} attribute. + * only forwards variables named in its {@code capture} attribute. Fixed once, + * for every caller, in {@code lib/form/dropdownDescriptorSelector.jelly} + * itself rather than on this page individually. */ @WithJenkins class GlobalSecurityConfigurationReadOnlyModeTest { diff --git a/test/src/test/java/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest.java b/test/src/test/java/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest.java index 555a349ce326..dbb273f8a158 100644 --- a/test/src/test/java/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest.java +++ b/test/src/test/java/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest.java @@ -23,9 +23,12 @@ * Reproduces JENKINS-12548: the SCM Checkout Strategy dropdown in * {@code config-scm.jelly} renders non-selected descriptors lazily via * {@code l:renderOnDemand}, which only forwards variables named in its - * {@code capture} attribute. Without {@code capture="readOnlyMode"}, a - * viewer with {@link Item#EXTENDED_READ} but not {@link Item#CONFIGURE} - * gets an editable field once that fragment is loaded. + * {@code capture} attribute. Before the fix in + * {@code lib/form/dropdownDescriptorSelector.jelly} (which now always + * captures {@code readOnlyMode}, alongside the already-always-captured + * {@code descriptor}/{@code it}), a viewer with {@link Item#EXTENDED_READ} + * but not {@link Item#CONFIGURE} got an editable field once that fragment + * was lazily loaded. */ @WithJenkins class SCMCheckoutStrategyReadOnlyModeTest { From e40de5064bb3bc56d08846ec0559d33f1f298642 Mon Sep 17 00:00:00 2001 From: Aitor Lopez Date: Mon, 3 Aug 2026 12:50:21 +0200 Subject: [PATCH 5/6] Add a taglib-level regression test for dropdownDescriptorSelector The two existing regression tests on this branch (SCMCheckoutStrategy, GlobalSecurityConfiguration) exercise real pages end-to-end through JenkinsRule + MockAuthorizationStrategy, which only covers 2 of the now-generic fix's many callers and requires standing up a full permission model to prove a taglib-level behavior. Following this project's existing convention for testing this exact taglib family (lib/form/DropdownListTest, lib/layout/RenderOnDemandTest, lib/form/RowVisibilityGroupTest, the latter of which already reuses a single config.jelly across sibling Descriptor implementations), add a lean unit test that exercises f:dropdownDescriptorSelector directly via a synthetic RootAction, with no security realm involved. The fixture's test1.jelly sets readOnlyMode directly and deliberately omits capture="readOnlyMode", proving the taglib itself is responsible for propagating it into the lazily-rendered fragment -- which by construction covers every current and future caller, not just the two pages exercised by the other tests. --- .../form/DropdownDescriptorSelectorTest.java | 114 ++++++++++++++++++ .../Fruit/config.jelly | 7 ++ .../RootActionImpl/test1.jelly | 16 +++ 3 files changed, 137 insertions(+) create mode 100644 test/src/test/java/lib/form/DropdownDescriptorSelectorTest.java create mode 100644 test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/Fruit/config.jelly create mode 100644 test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/RootActionImpl/test1.jelly diff --git a/test/src/test/java/lib/form/DropdownDescriptorSelectorTest.java b/test/src/test/java/lib/form/DropdownDescriptorSelectorTest.java new file mode 100644 index 000000000000..508e0fe012cd --- /dev/null +++ b/test/src/test/java/lib/form/DropdownDescriptorSelectorTest.java @@ -0,0 +1,114 @@ +package lib.form; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import hudson.model.Describable; +import hudson.model.Descriptor; +import hudson.model.InvisibleAction; +import hudson.model.RootAction; +import java.util.Objects; +import jenkins.model.Jenkins; +import org.htmlunit.WebClientUtil; +import org.htmlunit.html.DomElement; +import org.htmlunit.html.HtmlPage; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.TestExtension; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; +import org.kohsuke.stapler.DataBoundConstructor; + +/** + * Tests that {@code f:dropdownDescriptorSelector} always captures {@code readOnlyMode} into + * the lazily-rendered ({@code l:renderOnDemand}) fragment of a non-selected descriptor, without + * the caller having to opt in via the taglib's {@code capture} attribute. This exercises the + * taglib mechanism directly (see {@code test1.jelly}, which does not pass {@code capture}), + * independently of any specific page's permission model. + */ +@WithJenkins +class DropdownDescriptorSelectorTest { + + private JenkinsRule j; + + @BeforeEach + void setUp(JenkinsRule rule) { + j = rule; + } + + @Issue("JENKINS-12548") + @Test + void readOnlyModeIsCapturedWithoutCallerOptingIn() throws Exception { + HtmlPage p = j.createWebClient().goTo("self/test1"); + + // Triggers the same client-side fetch normally fired when the user picks + // a non-default option from the dropdown. + p.executeJavaScript( + "document.getElementsBySelector('.render-on-demand').forEach(function(e) { renderOnDemand(e); })"); + WebClientUtil.waitForJSExec(p.getWebClient()); + + DomElement fruitBlock = p.querySelector("div[name='fruit']"); + assertNotNull(fruitBlock, "expected the dropdown block to be rendered"); + + assertNull(fruitBlock.querySelector("input[name='_.value']"), + "the lazily-loaded, non-selected descriptor fragment must not expose an editable 'value' field" + + " when readOnlyMode is true, even though test1.jelly's f:dropdownDescriptorSelector" + + " does not pass capture=\"readOnlyMode\""); + assertNotNull(fruitBlock.querySelector(".jenkins-not-applicable"), + "read-only mode should instead render the standard N/A read-only placeholder"); + } + + public abstract static class Fruit implements Describable { + private final String value; + + protected Fruit(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + } + + public static class Apple extends Fruit { + @SuppressWarnings("checkstyle:redundantmodifier") + @DataBoundConstructor + public Apple(String value) { + super(value); + } + + @TestExtension + public static class DescriptorImpl extends Descriptor {} + } + + public static class Banana extends Fruit { + @SuppressWarnings("checkstyle:redundantmodifier") + @DataBoundConstructor + public Banana(String value) { + super(value); + } + + @TestExtension + public static class DescriptorImpl extends Descriptor {} + } + + @TestExtension + public static final class RootActionImpl extends InvisibleAction implements Describable, RootAction { + + public Fruit fruit = new Apple("seed"); + + @Override + public Descriptor getDescriptor() { + return Objects.requireNonNull(Jenkins.get().getDescriptorByType(DescriptorImpl.class)); + } + + @TestExtension + public static final class DescriptorImpl extends Descriptor {} + + @Override + public String getUrlName() { + return "self"; + } + } +} diff --git a/test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/Fruit/config.jelly b/test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/Fruit/config.jelly new file mode 100644 index 000000000000..69a00c9b4117 --- /dev/null +++ b/test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/Fruit/config.jelly @@ -0,0 +1,7 @@ + + + + + + + diff --git a/test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/RootActionImpl/test1.jelly b/test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/RootActionImpl/test1.jelly new file mode 100644 index 000000000000..72bb54575eab --- /dev/null +++ b/test/src/test/resources/lib/form/DropdownDescriptorSelectorTest/RootActionImpl/test1.jelly @@ -0,0 +1,16 @@ + + + + + + + + + + + + + + + From e4dc15f2566e6bcd2537fa61331cafac6be6f278 Mon Sep 17 00:00:00 2001 From: Aitor Moya Lopez <41477241+Racknaraock@users.noreply.github.com> Date: Sat, 8 Aug 2026 11:14:19 +0200 Subject: [PATCH 6/6] Empty commit to retrigger CI