Skip to content

Propagate readOnlyMode into all dropdownDescriptorSelector fragments in core - #27167

Open
Racknaraock wants to merge 7 commits into
jenkinsci:masterfrom
Racknaraock:JENKINS-12548-scm-checkout-strategy-readonly
Open

Propagate readOnlyMode into all dropdownDescriptorSelector fragments in core#27167
Racknaraock wants to merge 7 commits into
jenkinsci:masterfrom
Racknaraock:JENKINS-12548-scm-checkout-strategy-readonly

Conversation

@Racknaraock

@Racknaraock Racknaraock commented Jul 30, 2026

Copy link
Copy Markdown

Part of JENKINS-12548 "Read-only system configuration browsing" (#12412), the JEP-224 effort to make config/admin pages properly disable themselves for users with Item.EXTENDED_READ/Overall.SYSTEM_READ but without CONFIGURE/ADMINISTER.

No individual JENKINS issue exists yet for this specific gap (a generic core issue, not previously filed); per CONTRIBUTING.md this is treated as a minor improvement/bugfix that doesn't require one.

Background / how this was found

f:dropdownDescriptorSelector (lib/form/dropdownDescriptorSelector.jelly) renders the currently-selected option inline, but defers every other option's config.jelly/config.groovy via l:renderOnDemand, fetched later through a separate AJAX/stapler-proxy call. renderOnDemand only forwards variables named in its capture attribute into that later render. readOnlyMode (the variable pages like Job/configure.jelly set to drive read-only rendering) was never in any of these capture lists, so a viewer with only Item.EXTENDED_READ/Overall.SYSTEM_READ could get back a fully editable-looking fragment for any option other than the currently-selected one — even though every other field on the same page correctly disables.

This was first discovered and fixed for the "SCM Checkout Strategy" dropdown on Job/configure.jelly while working on cloudbees-folder-plugin#747 (JENKINS-62218, read-only folder configuration) — that PR originally carried a folder-specific version of this same fix, which was reverted there once it became clear the root cause lives in core and affects every Job configure page, not just folders. @jtnord's review on that PR confirmed this "[w]ould appear to be a generic bug in Jenkins core and would affect all users of dropdownDescriptorSelector across the project," which prompted auditing every *.jelly/*.groovy usage of the tag in core/ to find and close the remaining instances rather than fixing only the one that was originally reported.

What this PR fixes

Adds capture="readOnlyMode" (or capture: "readOnlyMode" in Groovy views) to every remaining f:dropdownDescriptorSelector call site whose containing page actually sets readOnlyMode:

  • HudsonPrivateSecurityRealm/config.jelly — Password Complexity Rule
  • DelegatingComputerLauncher/config.jelly, 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, 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
  • GlobalSecurityConfiguration/index.groovy — Security Realm, Authorization, Markup Formatter selectors on "Manage Jenkins > Security" (reachable with only Overall.SYSTEM_READ)

hudson/model/MyViewsProperty/config.jelly (Views Tab Bar on a user's own account page) was deliberately left out: that page (UserPropertyCategoryPreferencesAction) is gated by Jenkins.ADMINISTER checked against the nearest AccessControlled ancestor, which resolves to the target User, and User#getACL() always grants a non-anonymous user full control of themselves regardless of any Jenkins-wide role. So this page is only ever reachable by the account owner (full access, by design) or a real instance administrator -- there is no partial/read-only view of someone else's account settings, and readOnlyMode is never set anywhere in its render chain. Not an instance of this bug.

A note on GlobalSecurityConfiguration specifically, since it's a more sensitive page than the others: before including it here, I verified this is not a privilege-escalation vector. GlobalSecurityConfiguration#configure() calls j.checkPermission(Jenkins.ADMINISTER) unconditionally as its first line, before any submitted JSON is read, so a forged configSubmit POST from a SYSTEM_READ-only user is rejected server-side (AccessDeniedException3: ... is missing the Overall/Administer permission) 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. The same double gate (checkPermission(CONFIGURE) in doConfigSubmit) exists for the Job/Computer-scoped sites above, so all of these are the same severity class: a read-only-mode UI/UX gap, not a write-path vulnerability.

Testing done

Added two regression tests:

  • test/src/test/java/jenkins/scm/SCMCheckoutStrategyReadOnlyModeTest.java — registers a throwaway SCMCheckoutStrategy with a real editable field, loads a Job's configure page as a viewer with only Item.READ+Item.EXTENDED_READ, triggers the same client-side renderOnDemand() JS a browser fires when switching the dropdown, and asserts the resulting fragment renders as the standard read-only N/A placeholder rather than an editable <input>. Verified manually that this test fails (editable <input name="_.value">) against the pre-fix config-scm.jelly and passes (read-only placeholder) against the fix.
  • test/src/test/java/hudson/security/GlobalSecurityConfigurationReadOnlyModeTest.java — same pattern, covering the architecturally distinct Groovy-view/SYSTEM_READ path via a throwaway AuthorizationStrategy.

The other 8 call sites apply the identical one-line change against the identical capture/renderOnDemand mechanism exercised by both tests above, but don't have their own dedicated regression test; happy to add more if reviewers want per-site coverage.

Screenshots (UI changes only)

Nothing changes visually for users who already have CONFIGURE/ADMINISTER. For a viewer with only Overall.SYSTEM_READ, captured on the "Password Complexity Rule" dropdown (HudsonPrivateSecurityRealm/config.jelly) as a representative example -- same mechanism as the rest of this PR.

Before: Minimum password length renders as a live <input type="number" value="8">, and the four Require at least one ... checkboxes have no disabled attribute at all.

before_bug_authorization_strategy

After: Minimum password length renders as <pre class="jenkins-readonly">8</pre> (the standard core read-only placeholder), and all four checkboxes carry disabled="true".

after_fixed_authorization_strategy

Proposed changelog entries

  • Fix several dropdownDescriptorSelector-based configuration fields (SCM Checkout Strategy on Job pages, agent launch method/retention strategy, Maven settings providers, fingerprint storage engine, discard-old-builds strategy, views tab bar, crumb issuer, password complexity rule, Security Realm/Authorization/Markup Formatter on the Security page) not respecting read-only mode for viewers with Item.EXTENDED_READ/Overall.SYSTEM_READ

Proposed changelog category

/label bug

Proposed upgrade guidelines

N/A

Submitter checklist

  • The issue, if it exists, is well-described. (No individual issue filed; described above and linked to epic JENKINS-12548 / [JENKINS-12548] Read-only system configuration browsing #12412.)
  • The changelog entry is appropriate for the audience affected (users) and in the imperative mood.
  • There is automated testing (see Testing done).
  • New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate. (N/A — no new public API.)
  • New deprecations are annotated. (N/A — no deprecations.)
  • UI changes do not introduce CSP regressions (attribute-only change to existing tags, no new/changed JS).
  • For dependency updates... (N/A.)
  • For new APIs and extension points... (N/A.)

Desired reviewers

@jenkinsci/core-pr-reviewers

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.
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.
@comment-ops-bot comment-ops-bot Bot added the bug For changelog: Minor bug. Will be listed after features label Jul 30, 2026
…iptorSelector 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.
@Racknaraock Racknaraock changed the title Propagate readOnlyMode into remaining dropdownDescriptorSelector fragments Propagate readOnlyMode into all dropdownDescriptorSelector fragments in core Jul 30, 2026
@timja

timja commented Jul 30, 2026

Copy link
Copy Markdown
Member

Screenshots are required for any visual change. Please demonstrate what it looks like for users with read only access.

@jtnord jtnord left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather than change all callers I was suggesting that propagating readOnlyMode should be done always in the taglib. That is it should not need changes across the ecosystem for any page showing a readonly mode using dropdownDescriptorSelector

descriptors: all,
field: 'crumbIssuer'
field: 'crumbIssuer',
capture: "readOnlyMode"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather than change all callers I was suggesting that propagating readOnlyMode should be done always in the taglib. That is it should not need changes across the ecosystem for any page showing a readonly mode using dropdownDescriptorSelector

@Racknaraock Racknaraock Aug 3, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, thanks — done in 061162e.

… per caller

Per jtnord's review on jenkinsci#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.
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.
@Racknaraock

Racknaraock commented Aug 3, 2026

Copy link
Copy Markdown
Author

Screenshots are required for any visual change. Please demonstrate what it looks like for users with read only access.

@timja

I've only included one of the screenshots because, based on jtnord's comments, the changes apply across the entire structure. Let me know if this is okay or if you need all the screenshots of the affected components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug For changelog: Minor bug. Will be listed after features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants