From 0bc0fda4bf9db101d3af3199e8835e96992f37d2 Mon Sep 17 00:00:00 2001 From: Aitor Lopez Date: Sat, 25 Jul 2026 13:33:15 +0200 Subject: [PATCH 1/7] JENKINS-62218: Expand Health metrics by default for read-only viewers f:advanced collapses the Health metrics section behind a click-to-expand toggle regardless of permissions. A user with Item.READ/EXTENDED_READ but not Item.CONFIGURE has no way to interact with the form anyway, so hiding already-configured health metrics behind an extra click only makes the read-only view less informative, contrary to the read-only rendering convention described in JENKINS-12548 (every advanced section should be expanded out of the box for read-only browsing). When readOnlyMode is true, render the health metrics hetero-list directly instead of wrapping it in f:advanced. Behavior for users with CONFIGURE is unchanged. --- .../folder/AbstractFolder/configure.jelly | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly index 4837bf60..2d6b8256 100644 --- a/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly +++ b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly @@ -128,14 +128,26 @@ THE SOFTWARE. - - - - - + + + + + + + + + + + + + + From 67b52741bb291b6ef5d44c27ee45851fd94ea0e9 Mon Sep 17 00:00:00 2001 From: Aitor Lopez Date: Sat, 25 Jul 2026 13:42:12 +0200 Subject: [PATCH 2/7] JENKINS-62218: Add regression test for read-only health metrics rendering Covers both cases: a viewer with Item.READ/EXTENDED_READ but not Item.CONFIGURE sees the configured health metric directly without an Advanced toggle, while a user with CONFIGURE still gets the collapsible Advanced section as before. --- .../hudson/plugins/folder/FolderTest.java | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java b/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java index 00c1194b..8d96ad35 100644 --- a/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java +++ b/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java @@ -31,6 +31,7 @@ import com.cloudbees.hudson.plugins.folder.config.AbstractFolderConfiguration; import com.cloudbees.hudson.plugins.folder.health.FolderHealthMetric; import com.cloudbees.hudson.plugins.folder.health.FolderHealthMetricDescriptor; +import com.cloudbees.hudson.plugins.folder.health.WorstChildHealthMetric; import com.cloudbees.hudson.plugins.folder.properties.FolderCredentialsProvider; import com.cloudbees.plugins.credentials.domains.DomainCredentials; import hudson.model.AbstractItem; @@ -631,4 +632,41 @@ void doCreateItem() throws Exception { // The request sent is using a GET instead of POST request which is not allowed assertEquals(405, webClient.goTo(folderURL).getWebResponse().getStatusCode()); } + + @Issue("JENKINS-62218") + @Test + void readOnlyViewerSeesExpandedHealthMetrics() throws Exception { + Folder f = createFolder(); + f.getHealthMetrics().add(new WorstChildHealthMetric()); + f.save(); + + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); + mockStrategy + .grant(Jenkins.READ, Item.READ, Item.EXTENDED_READ) + .everywhere() + .to("viewer"); + mockStrategy.grant(Jenkins.ADMINISTER).everywhere().to("admin"); + r.jenkins.setAuthorizationStrategy(mockStrategy); + + JenkinsRule.WebClient viewer = r.createWebClient(); + viewer.login("viewer"); + HtmlPage viewerConfigure = viewer.getPage(f, "configure"); + assertThat( + "a read-only viewer should not need to click an Advanced toggle to see configured health metrics", + viewerConfigure.getByXPath("//button[contains(@class, 'advancedButton')]"), + is(empty())); + assertThat( + "the configured health metric should be visible without expanding anything", + viewerConfigure.asNormalizedText(), + containsString("Child item with worst health")); + + JenkinsRule.WebClient admin = r.createWebClient(); + admin.login("admin"); + HtmlPage adminConfigure = admin.getPage(f, "configure"); + assertThat( + "the Advanced toggle must still be rendered for a user who can configure the folder", + adminConfigure.getByXPath("//button[contains(@class, 'advancedButton')]"), + not(empty())); + } } From c9aed80e7ff0a86559209f36ec22b9176ef29284 Mon Sep 17 00:00:00 2001 From: Aitor Lopez Date: Mon, 27 Jul 2026 11:11:06 +0200 Subject: [PATCH 3/7] JENKINS-62218: Disable Default View select for read-only viewers The Appearance section's Default View + is bypassed client-side or the render-on-demand endpoint is hit directly. Add capture="readOnlyMode" to both dropdownDescriptorSelector call sites (viewsTabBar and icon). Add a regression test that registers a second fake FolderIcon descriptor via @TestExtension so the Icon dropdown has a non-selected entry to lazily render, fetches that lazy fragment through the same stapler-bound-proxy protocol the browser's own JavaScript uses, and asserts a read-only viewer sees the read-only placeholder instead of an editable . Verified the test fails without the capture attribute and passes with it. --- .../folder/AbstractFolder/configure.jelly | 10 +- .../hudson/plugins/folder/FolderTest.java | 113 ++++++++++++++++++ .../FakeCaptureFolderIcon/config.jelly | 12 ++ 3 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 src/test/resources/com/cloudbees/hudson/plugins/folder/FolderTest/FakeCaptureFolderIcon/config.jelly diff --git a/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly index ae7bead3..850ebe37 100644 --- a/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly +++ b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly @@ -97,11 +97,11 @@ THE SOFTWARE. - + - + @@ -109,11 +109,11 @@ THE SOFTWARE. - + - + @@ -122,7 +122,7 @@ THE SOFTWARE. - + diff --git a/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java b/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java index 8d96ad35..8420c335 100644 --- a/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java +++ b/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java @@ -76,6 +76,7 @@ import jenkins.model.RenameAction; import jenkins.util.Timer; import org.htmlunit.HttpMethod; +import org.htmlunit.Page; import org.htmlunit.WebRequest; import org.htmlunit.html.*; import org.junit.jupiter.api.BeforeEach; @@ -89,6 +90,7 @@ import org.jvnet.hudson.test.junit.jupiter.BuildWatcherExtension; import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import org.jvnet.hudson.test.recipes.LocalData; +import org.kohsuke.stapler.DataBoundConstructor; import org.springframework.security.access.AccessDeniedException; @WithJenkins @@ -669,4 +671,115 @@ void readOnlyViewerSeesExpandedHealthMetrics() throws Exception { adminConfigure.getByXPath("//button[contains(@class, 'advancedButton')]"), not(empty())); } + + /** + * {@code f:dropdownDescriptorSelector} renders the config fragment of the currently selected + * descriptor inline, but every other candidate descriptor's fragment is rendered lazily via + * {@code l:renderOnDemand}. {@code RenderOnDemandClosure} snapshots only the jelly variables + * named in the tag's {@code capture} attribute (plus a few Stapler well-known bindings) at the + * time the outer page is rendered; anything else, including the outer {@code readOnlyMode} + * variable set in {@code AbstractFolder/configure.jelly}, is invisible to the lazily-rendered + * fragment unless explicitly captured. This test registers a second {@link FolderIcon} + * descriptor (so the Icon dropdown actually needs to lazily render a non-selected entry) and + * fetches that lazy fragment directly through the same stapler-bound-proxy protocol the + * browser's own JavaScript uses (see {@code org/kohsuke/stapler/bind.js}), to assert that a + * read-only viewer sees the field as read-only there too. + */ + @Issue("JENKINS-62218") + @Test + void readOnlyViewerCannotEditLazyIconDescriptorFields() throws Exception { + Folder f = createFolder(); + f.save(); + + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); + mockStrategy + .grant(Jenkins.READ, Item.READ, Item.EXTENDED_READ) + .everywhere() + .to("viewer"); + mockStrategy.grant(Jenkins.ADMINISTER).everywhere().to("admin"); + r.jenkins.setAuthorizationStrategy(mockStrategy); + + JenkinsRule.WebClient viewer = r.createWebClient(); + viewer.login("viewer"); + HtmlPage viewerConfigure = viewer.getPage(f, "configure"); + + List lazyBlocks = viewerConfigure.getByXPath("//div[contains(@class, 'render-on-demand')]"); + assertThat( + "the folder has two icon descriptors (stock + the fake one registered below) and only" + + " the non-selected one (the fake one) should need to be rendered lazily", + lazyBlocks, + hasSize(1)); + DomNode lazyBlock = lazyBlocks.get(0); + String proxyUrl = lazyBlock.getAttributes().getNamedItem("data-proxy-url").getNodeValue(); + String crumb = lazyBlock.getAttributes().getNamedItem("data-proxy-crumb").getNodeValue(); + + if (!proxyUrl.endsWith("/")) { + proxyUrl += "/"; + } + URL renderUrl = new URL(viewerConfigure.getUrl(), proxyUrl + "render"); + WebRequest renderRequest = new WebRequest(renderUrl, HttpMethod.POST); + renderRequest.setAdditionalHeader("Content-Type", "application/x-stapler-method-invocation;charset=UTF-8"); + renderRequest.setAdditionalHeader("Crumb", crumb); + // bind.js also sends this header, populated client-side by Jenkins' own crumb.js wrapper + // (crumb.wrap(headers)) using the crumb issuer's actual configured field name; replicate + // that here since we are not executing the real browser-side JavaScript. + renderRequest.setAdditionalHeader( + r.jenkins.getCrumbIssuer().getCrumbRequestField(), crumb); + renderRequest.setRequestBody("[]"); + Page renderedFragment = viewer.getPage(renderRequest); + String html = renderedFragment.getWebResponse().getContentAsString(); + + assertThat( + "a read-only viewer must not see an editable for a field belonging to a" + + " lazily-loaded (non-selected) descriptor fragment", + html, + not(containsString(" + + + + + + From 88edadd07fcaffaa60d57f6969c9e76980a30acb Mon Sep 17 00:00:00 2001 From: Aitor Lopez Date: Mon, 27 Jul 2026 12:08:43 +0200 Subject: [PATCH 5/7] JENKINS-62218: Fix Spotless formatting in FolderTest --- .../com/cloudbees/hudson/plugins/folder/FolderTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java b/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java index 8420c335..eef9ba90 100644 --- a/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java +++ b/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java @@ -711,8 +711,10 @@ void readOnlyViewerCannotEditLazyIconDescriptorFields() throws Exception { lazyBlocks, hasSize(1)); DomNode lazyBlock = lazyBlocks.get(0); - String proxyUrl = lazyBlock.getAttributes().getNamedItem("data-proxy-url").getNodeValue(); - String crumb = lazyBlock.getAttributes().getNamedItem("data-proxy-crumb").getNodeValue(); + String proxyUrl = + lazyBlock.getAttributes().getNamedItem("data-proxy-url").getNodeValue(); + String crumb = + lazyBlock.getAttributes().getNamedItem("data-proxy-crumb").getNodeValue(); if (!proxyUrl.endsWith("/")) { proxyUrl += "/"; @@ -724,8 +726,7 @@ void readOnlyViewerCannotEditLazyIconDescriptorFields() throws Exception { // bind.js also sends this header, populated client-side by Jenkins' own crumb.js wrapper // (crumb.wrap(headers)) using the crumb issuer's actual configured field name; replicate // that here since we are not executing the real browser-side JavaScript. - renderRequest.setAdditionalHeader( - r.jenkins.getCrumbIssuer().getCrumbRequestField(), crumb); + renderRequest.setAdditionalHeader(r.jenkins.getCrumbIssuer().getCrumbRequestField(), crumb); renderRequest.setRequestBody("[]"); Page renderedFragment = viewer.getPage(renderRequest); String html = renderedFragment.getWebResponse().getContentAsString(); From 70b6c69e06cc34f49d6cd9e42ff943833f2bd657 Mon Sep 17 00:00:00 2001 From: Aitor Lopez Date: Mon, 27 Jul 2026 13:33:11 +0200 Subject: [PATCH 6/7] JENKINS-62218: Revert capture=readOnlyMode fix for lazy tab bar/icon descriptors The underlying gap (l:renderOnDemand's RenderOnDemandClosure only exposes variables named in `capture`, so readOnlyMode set in an ancestor scope is invisible to lazily-rendered descriptor fragments) is not specific to this plugin: Jenkins core's own hudson/model/Job/configure.jelly sets readOnlyMode the same way and includes config-scm.jelly, whose dropdownDescriptorSelector for scmCheckoutStrategy has the identical gap. Per review feedback, fixing this locally in cloudbees-folder-plugin wouldn't address the same issue on every other Job configuration page, so this should be fixed in Jenkins core instead. Reverts the capture="readOnlyMode" attribute additions and the associated FakeCaptureFolderIcon test. This is a revert of e6eed4f and 88edadd; the Default View select fix (c9aed80) and Health metrics fix (0bc0fda) are unaffected. --- .../folder/AbstractFolder/configure.jelly | 10 +- .../hudson/plugins/folder/FolderTest.java | 114 ------------------ .../FakeCaptureFolderIcon/config.jelly | 12 -- 3 files changed, 5 insertions(+), 131 deletions(-) delete mode 100644 src/test/resources/com/cloudbees/hudson/plugins/folder/FolderTest/FakeCaptureFolderIcon/config.jelly diff --git a/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly index 850ebe37..ae7bead3 100644 --- a/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly +++ b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly @@ -97,11 +97,11 @@ THE SOFTWARE. - + - + @@ -109,11 +109,11 @@ THE SOFTWARE. - + - + @@ -122,7 +122,7 @@ THE SOFTWARE. - + diff --git a/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java b/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java index eef9ba90..8d96ad35 100644 --- a/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java +++ b/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java @@ -76,7 +76,6 @@ import jenkins.model.RenameAction; import jenkins.util.Timer; import org.htmlunit.HttpMethod; -import org.htmlunit.Page; import org.htmlunit.WebRequest; import org.htmlunit.html.*; import org.junit.jupiter.api.BeforeEach; @@ -90,7 +89,6 @@ import org.jvnet.hudson.test.junit.jupiter.BuildWatcherExtension; import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import org.jvnet.hudson.test.recipes.LocalData; -import org.kohsuke.stapler.DataBoundConstructor; import org.springframework.security.access.AccessDeniedException; @WithJenkins @@ -671,116 +669,4 @@ void readOnlyViewerSeesExpandedHealthMetrics() throws Exception { adminConfigure.getByXPath("//button[contains(@class, 'advancedButton')]"), not(empty())); } - - /** - * {@code f:dropdownDescriptorSelector} renders the config fragment of the currently selected - * descriptor inline, but every other candidate descriptor's fragment is rendered lazily via - * {@code l:renderOnDemand}. {@code RenderOnDemandClosure} snapshots only the jelly variables - * named in the tag's {@code capture} attribute (plus a few Stapler well-known bindings) at the - * time the outer page is rendered; anything else, including the outer {@code readOnlyMode} - * variable set in {@code AbstractFolder/configure.jelly}, is invisible to the lazily-rendered - * fragment unless explicitly captured. This test registers a second {@link FolderIcon} - * descriptor (so the Icon dropdown actually needs to lazily render a non-selected entry) and - * fetches that lazy fragment directly through the same stapler-bound-proxy protocol the - * browser's own JavaScript uses (see {@code org/kohsuke/stapler/bind.js}), to assert that a - * read-only viewer sees the field as read-only there too. - */ - @Issue("JENKINS-62218") - @Test - void readOnlyViewerCannotEditLazyIconDescriptorFields() throws Exception { - Folder f = createFolder(); - f.save(); - - r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); - MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); - mockStrategy - .grant(Jenkins.READ, Item.READ, Item.EXTENDED_READ) - .everywhere() - .to("viewer"); - mockStrategy.grant(Jenkins.ADMINISTER).everywhere().to("admin"); - r.jenkins.setAuthorizationStrategy(mockStrategy); - - JenkinsRule.WebClient viewer = r.createWebClient(); - viewer.login("viewer"); - HtmlPage viewerConfigure = viewer.getPage(f, "configure"); - - List lazyBlocks = viewerConfigure.getByXPath("//div[contains(@class, 'render-on-demand')]"); - assertThat( - "the folder has two icon descriptors (stock + the fake one registered below) and only" - + " the non-selected one (the fake one) should need to be rendered lazily", - lazyBlocks, - hasSize(1)); - DomNode lazyBlock = lazyBlocks.get(0); - String proxyUrl = - lazyBlock.getAttributes().getNamedItem("data-proxy-url").getNodeValue(); - String crumb = - lazyBlock.getAttributes().getNamedItem("data-proxy-crumb").getNodeValue(); - - if (!proxyUrl.endsWith("/")) { - proxyUrl += "/"; - } - URL renderUrl = new URL(viewerConfigure.getUrl(), proxyUrl + "render"); - WebRequest renderRequest = new WebRequest(renderUrl, HttpMethod.POST); - renderRequest.setAdditionalHeader("Content-Type", "application/x-stapler-method-invocation;charset=UTF-8"); - renderRequest.setAdditionalHeader("Crumb", crumb); - // bind.js also sends this header, populated client-side by Jenkins' own crumb.js wrapper - // (crumb.wrap(headers)) using the crumb issuer's actual configured field name; replicate - // that here since we are not executing the real browser-side JavaScript. - renderRequest.setAdditionalHeader(r.jenkins.getCrumbIssuer().getCrumbRequestField(), crumb); - renderRequest.setRequestBody("[]"); - Page renderedFragment = viewer.getPage(renderRequest); - String html = renderedFragment.getWebResponse().getContentAsString(); - - assertThat( - "a read-only viewer must not see an editable for a field belonging to a" - + " lazily-loaded (non-selected) descriptor fragment", - html, - not(containsString(" - - - - - - From 853d481af5e7c7a950f1aa4b915bad69cb1b7e0e Mon Sep 17 00:00:00 2001 From: Aitor Lopez Date: Mon, 3 Aug 2026 17:13:23 +0200 Subject: [PATCH 7/7] JENKINS-62218: Revert Health metrics f:advanced expansion for read-only viewers Per review feedback from jtnord and timja, advanced configuration sections should render the same regardless of readOnlyMode: if a section was previously collapsed behind f:advanced, it should stay that way in read-only mode too, rather than auto-expanding just because the viewer lacks CONFIGURE. Only the Default View select's disabled attribute (c9aed80) remains from this PR's original scope. This is a revert of 0bc0fda and 67b5274. --- .../folder/AbstractFolder/configure.jelly | 28 ++++---------- .../hudson/plugins/folder/FolderTest.java | 38 ------------------- 2 files changed, 8 insertions(+), 58 deletions(-) diff --git a/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly index ae7bead3..45769009 100644 --- a/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly +++ b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly @@ -128,26 +128,14 @@ THE SOFTWARE. - - - - - - - - - - - - - - + + + + + diff --git a/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java b/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java index 8d96ad35..00c1194b 100644 --- a/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java +++ b/src/test/java/com/cloudbees/hudson/plugins/folder/FolderTest.java @@ -31,7 +31,6 @@ import com.cloudbees.hudson.plugins.folder.config.AbstractFolderConfiguration; import com.cloudbees.hudson.plugins.folder.health.FolderHealthMetric; import com.cloudbees.hudson.plugins.folder.health.FolderHealthMetricDescriptor; -import com.cloudbees.hudson.plugins.folder.health.WorstChildHealthMetric; import com.cloudbees.hudson.plugins.folder.properties.FolderCredentialsProvider; import com.cloudbees.plugins.credentials.domains.DomainCredentials; import hudson.model.AbstractItem; @@ -632,41 +631,4 @@ void doCreateItem() throws Exception { // The request sent is using a GET instead of POST request which is not allowed assertEquals(405, webClient.goTo(folderURL).getWebResponse().getStatusCode()); } - - @Issue("JENKINS-62218") - @Test - void readOnlyViewerSeesExpandedHealthMetrics() throws Exception { - Folder f = createFolder(); - f.getHealthMetrics().add(new WorstChildHealthMetric()); - f.save(); - - r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); - MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); - mockStrategy - .grant(Jenkins.READ, Item.READ, Item.EXTENDED_READ) - .everywhere() - .to("viewer"); - mockStrategy.grant(Jenkins.ADMINISTER).everywhere().to("admin"); - r.jenkins.setAuthorizationStrategy(mockStrategy); - - JenkinsRule.WebClient viewer = r.createWebClient(); - viewer.login("viewer"); - HtmlPage viewerConfigure = viewer.getPage(f, "configure"); - assertThat( - "a read-only viewer should not need to click an Advanced toggle to see configured health metrics", - viewerConfigure.getByXPath("//button[contains(@class, 'advancedButton')]"), - is(empty())); - assertThat( - "the configured health metric should be visible without expanding anything", - viewerConfigure.asNormalizedText(), - containsString("Child item with worst health")); - - JenkinsRule.WebClient admin = r.createWebClient(); - admin.login("admin"); - HtmlPage adminConfigure = admin.getPage(f, "configure"); - assertThat( - "the Advanced toggle must still be rendered for a user who can configure the folder", - adminConfigure.getByXPath("//button[contains(@class, 'advancedButton')]"), - not(empty())); - } }