Skip to content

Issue 36151 implement new site/folder field component edit contentlet#36308

Merged
freddyDOTCMS merged 25 commits into
mainfrom
issue-36151-Implement-new-site/folder-field-component-Edit-Contentlet
Jun 24, 2026
Merged

Issue 36151 implement new site/folder field component edit contentlet#36308
freddyDOTCMS merged 25 commits into
mainfrom
issue-36151-Implement-new-site/folder-field-component-Edit-Contentlet

Conversation

@freddyDOTCMS

@freddyDOTCMS freddyDOTCMS commented Jun 24, 2026

Copy link
Copy Markdown
Member

Proposed Changes

  • Add GET /api/v1/folder/search endpoint supporting optional case-insensitive name filter (min 3 chars), path scope, recursive depth control, pagination (page/per_page), and sort (name / mod_date) — replaces the deprecated POST /byPath
  • Deprecate POST /byPath (@Deprecated(forRemoval = true)) and mark it as deprecated in the OpenAPI spec; existing callers continue to work unchanged
  • Introduce FolderSearchParams record with a fluent builder to consolidate all search parameters across FolderAPI, FolderFactory, and FolderSearchPaginator, replacing a 10-argument method signature
  • Convert FolderSearchResultView from a plain class to a Java record
  • Add PermissionAPI.filterCollection(Collection<P>, int, User, boolean) — a batch permission check that resolves the entire collection in one SQL round-trip, eliminating the N+1 permission queries that caused ~2s response times on large sites
  • Add PermissionBitFactory.getPermittedIds() as the SQL backbone for the batch check, using a UNION of direct (permission) and inherited (permission_reference) permissions chunked in batches of 500

Checklist

  • Tests (unit: FolderSearchPaginatorTest, PermissionBitAPIImplFilterCollectionTest, PermissionBitFactoryImplGetPermittedIdsTest; integration: FolderAPIImplFilterTest, FolderFactoryImplFilterTest, FolderResourceSearchTest)
  • Translations
  • Security Implications Contemplated — all search results are permission-filtered via the new batch method; admin and system-user fast-paths are maintained; siteId is required and validated server-side before any DB query runs; role IDs injected into SQL are system-generated UUIDs (not user input), consistent with the existing filterCollectionByDBPermissionReference pattern in PermissionBitFactoryImpl

Additional Info

The deprecated POST /byPath endpoint is kept intact with backward-compatible pagination parameters (offset / limit) to avoid breaking existing integrations. The new endpoint uses the standard dotCMS pagination contract (page / per_page) and returns a ResponseEntityPaginatedDataView with full pagination metadata.

The batch permission approach reduces query count from O(N) — one DB/cache lookup per folder — to O(1) regardless of result set size, which is the primary fix for the 2-second response times reported on large sites.

Screenshots

N/A — backend API changes only

This PR fixes: #36151

This PR fixes: #36151

freddyDOTCMS and others added 24 commits May 8, 2026 14:02
…-new-site/folder-field-component-(Edit-Contentlet)
…Edit-Contentlet)' of https://github.com/dotCMS/core into issue-36151-Implement-new-site/folder-field-component-(Edit-Contentlet)
…Edit-Contentlet)' of https://github.com/dotCMS/core into issue-36151-Implement-new-site/folder-field-component-(Edit-Contentlet)
@github-actions github-actions Bot added Area : Backend PR changes Java/Maven backend code Area : Documentation PR changes documentation files labels Jun 24, 2026
@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

  • 🔴 Critical: dotCMS/src/main/java/com/dotmarketing/business/PermissionBitFactoryImpl.java:2735 — SQL injection vulnerability: sql string is built with direct string concatenation of user-controlled orderBy and orderDirection values without validation or parameterization. This allows SQL injection via orderby/direction query parameters.
  • 🔴 Critical: dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderFactoryImpl.java:1185 — SQL injection vulnerability: orderBy and orderDirection values from FolderSearchParams are directly interpolated into SQL without validation or parameterization. Attackers can inject arbitrary SQL via orderby/direction parameters.
  • 🟠 High: dotCMS/src/main/java/com/dotcms/rest/api/v1/folder/FolderResource.java:458validateSiteReadAccess throws BadRequestException for DotDataException, but DotDataException may indicate DB failure, not malformed siteId. This masks real DB errors as client errors, obscuring root cause and potentially hiding DoS or data corruption.
  • 🟠 High: dotCMS/src/main/java/com/dotmarketing/business/PermissionBitAPIImpl.java:1423filterCollection uses APILocator.systemUser() without checking if userIn is null — but userIn is already validated as non-null by getAnonymousUser() fallback. This logic is redundant and misleading; if userIn is null, getAnonymousUser() is called, but the subsequent user.getUserId() call may throw NullPointerException if getAnonymousUser() returns null (unhandled edge case).
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/util/pagination/FolderSearchPaginator.java:55extraParams may contain null values for siteId, path, or recursive; code assumes non-null but does not validate. This could cause ClassCastException or NullPointerException at runtime if frontend sends malformed JSON.
  • 🟡 Medium: dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderAPIImpl.java:828permissionAPI.filterCollection is called on the full result set before pagination, but the PaginatedArrayList returned by folderAPI.searchFolders may contain thousands of items. This defeats the purpose of pagination — memory usage scales with total matching folders, not page size. Risk of OOM on large sites.
  • 🟡 Medium: dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderFactoryImpl.java:1190normalizedPath logic appends / unconditionally if missing, but does not validate that path is a valid folder path (e.g., no .., no control chars). This could allow path traversal or malformed SQL if path contains ' or \0.
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/rest/api/v1/folder/FolderResource.java:464validateSiteReadAccess calls APILocator.getHostAPI().find(siteId, user, false) — but find may return a non-host Identifier (e.g., contentlet) if siteId is not a host inode. No type check — may silently return wrong object and allow access to non-host assets.

Existing

  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/rest/api/v1/folder/FolderResource.java:458 — Prior finding: DotDataException mapped to BadRequestException is a known anti-pattern in dotCMS codebase; this PR perpetuates it.

Resolved

  • dotCMS/src/main/java/com/dotcms/rest/api/v1/folder/FolderResource.java:458 — The validateSiteReadAccess method now explicitly checks for siteId validity and throws BadRequestException only for malformed input — but the underlying issue of misclassifying DotDataException as client error remains.

Run: #28118528869 · tokens: in: 20307 · out: 1350 · total: 21657

@claude

claude Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @freddyDOTCMS's task in 2m 21s —— View job


Rollback Safety Analysis

  • Read rollback-unsafe categories reference
  • Get full PR diff
  • Analyze diff against all unsafe categories
  • Post result and apply label

Result: ✅ Safe To Rollback

The label AI: Safe To Rollback has been applied.

Checked categories and findings:

Category Status Notes
C-1 Structural Data Model Change ✅ None No runonce tasks, no schema changes
C-2 ES Mapping Change ✅ None No ESMapping* files touched
C-3 Content JSON Version Bump ✅ None No CURRENT_MODEL_VERSION changes
C-4 DROP TABLE/COLUMN ✅ None No DDL drop statements
H-1 One-Way Data Migration ✅ None No data transforms
H-2 RENAME TABLE/COLUMN ✅ None None
H-3 PK Restructuring ✅ None None
H-4 New Field Type ✅ None None
H-5 Storage Provider Change ✅ None None
H-6 DROP PROCEDURE/FUNCTION ✅ None None
H-7 NOT NULL Column Without Default ✅ None No DDL at all
H-8 VTL Viewtool Contract Change ✅ None No viewtool classes modified
M-1 Column Type Change ✅ None No DDL
M-2 Push Publishing Bundle Format ✅ None No bundler changes
M-3 REST API Contract Change 🟢 LOW New GET /api/v1/folder/search endpoint is purely additive. POST /byPath is deprecated but its behavior is unchanged — callers continue to work. Any frontend that adopted the new endpoint during this release would get 404s on rollback, but N-1 itself is not broken.
M-4 OSGi Interface Change 🟢 LOW New methods added to PermissionAPI and FolderAPI interfaces. OSGi plugins that implement these interfaces (not just consume them) would fail to load on N-1, but these are internal consumer APIs with no known external implementors.

No CRITICAL or HIGH rollback risks found. Pure additions: new REST endpoint, new batch permission method, new factory method, new params record — all additive. The deprecated POST /byPath endpoint is kept intact and behaviorally unchanged.

@semgrep-dotcms

Copy link
Copy Markdown
Contributor

Semgrep found 1 CUSTOM_INJECTION-2 finding:

  • dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderFactoryImpl.java

The method identified is susceptible to injection. The input should be validated and properly
escaped.

If this is a critical or high severity finding, please also link this issue in the #security channel in Slack.

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

  • 🔴 Critical: dotCMS/src/main/java/com/dotmarketing/business/PermissionBitFactoryImpl.java:2735 — SQL injection vulnerability: user-controlled siteId, path, and orderBy values are directly concatenated into SQL string without parameterization or validation. The siteId and path are used in WHERE clauses with string concatenation, and orderBy is interpolated directly into SQL via String.format.
  • 🔴 Critical: dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderFactoryImpl.java:1185 — SQL injection vulnerability: orderBy and orderDirection values are directly interpolated into SQL via String.format without validation or parameterization. siteId and path are also concatenated into SQL without parameterization.
  • 🟠 High: dotCMS/src/main/java/com/dotcms/rest/api/v1/folder/FolderResource.java:492validateSiteReadAccess maps DotDataException to BadRequestException when siteId is invalid, masking database failures as client errors. This obscures server-side issues and violates the pattern flagged in prior review for FolderResource.java:458.
  • 🟠 High: dotCMS/src/main/java/com/dotcms/rest/api/v1/folder/FolderResource.java:492validateSiteReadAccess calls APILocator.getHostAPI().find(siteId, user, false) without validating siteId format (e.g., UUID structure), risking malformed SQL or unintended host lookup if siteId is non-numeric/non-UUID string.
  • 🟡 Medium: dotCMS/src/main/java/com/dotmarketing/business/PermissionBitFactoryImpl.java:2745getPermittedIds uses roleIds.forEach(dc::addParam) and chunk.forEach(dc::addParam) without checking for null values in roleIds or chunk, risking NullPointerException if any element is null.
  • 🟡 Medium: dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderFactoryImpl.java:1190 — Path normalization appends '/' unconditionally without validating path format, risking malformed SQL if path contains invalid characters or is empty.
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/util/pagination/FolderSearchPaginator.java:55extraParams may contain null values for siteId, path, or recursive; code assumes non-null without validation, risking ClassCastException or NullPointerException when casting to String or Boolean.
  • 🟡 Medium: dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderAPIImpl.java:828 — Permission filtering occurs on full result set before pagination, causing OOM risk on large folder sets. The searchFolders call returns all matching folders (no limit) before filtering and paginating in Java.
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/rest/api/v1/folder/FolderResource.java:492validateSiteReadAccess calls APILocator.getHostAPI().find(siteId, user, false) and checks site == null || !UtilMethods.isSet(site.getIdentifier()), but HostAPI.find may return non-host Identifier (e.g., contentlet), risking granting access to unintended assets.
  • 🟡 Medium: dotCMS/src/main/java/com/dotmarketing/business/PermissionBitAPIImpl.java:1423 — Redundant null check on userIn after getAnonymousUser() fallback; if getAnonymousUser() returns null, user.getUserId() throws NPE. user is assigned from userIn or getAnonymousUser(), but user.getUserId() is called unconditionally.
  • 🟡 Medium: dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderAPIImpl.java:837parentPath calculation assumes fullPath ends with folder name, but if folder name is empty or path is malformed, substring throws StringIndexOutOfBoundsException.
  • 🟡 Medium: dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderFactoryImpl.java:1190normalizedPath is computed as path + "/" if not ending in /, but if path is null, this throws NullPointerException.

Existing

  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/util/pagination/FolderSearchPaginator.java:55 — extraParams may contain null values for siteId, path, or recursive; code assumes non-null without validation, risking ClassCastException or NullPointerException.
  • 🟡 Medium: dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderAPIImpl.java:828 — Permission filtering occurs on full result set before pagination, causing OOM risk on large folder sets.
  • 🟡 Medium: dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderFactoryImpl.java:1190 — Path normalization appends '/' unconditionally without validating path format, risking path traversal or malformed SQL.
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/rest/api/v1/folder/FolderResource.java:464 — HostAPI.find may return non-host Identifier; no type check risks granting access to unintended assets.

Resolved

  • dotCMS/src/main/java/com/dotmarketing/business/PermissionBitAPIImpl.java:1423 — Prior finding about redundant null check on userIn after getAnonymousUser() fallback is resolved: code now checks userIn == null || userIn.getUserId() == null before fallback, and user.getUserId() is only called after ensuring user is non-null via fallback logic. The NPE risk is mitigated by the fallback guard.
  • dotCMS/src/main/java/com/dotmarketing/business/PermissionBitFactoryImpl.java:2735 — Prior SQL injection finding on orderBy/orderDirection is resolved: FolderFactoryImpl.searchFolders now validates orderBy against a whitelist (ALLOWED_SORT_COLUMNS) and falls back to default, eliminating direct interpolation. SQL is built with parameterized siteId and path via DotConnect.addParam().
  • dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderFactoryImpl.java:1185 — Prior SQL injection finding on orderBy/orderDirection is resolved: values are now validated against ALLOWED_SORT_COLUMNS and DEFAULT_SORT_DIRECTION, and SQL is built with parameterized inputs for siteId and path.

Run: #28119121091 · tokens: in: 20750 · out: 2162 · total: 22912

@mergify

mergify Bot commented Jun 24, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@freddyDOTCMS freddyDOTCMS added this pull request to the merge queue Jun 24, 2026
Merged via the queue into main with commit 6c92187 Jun 24, 2026
60 of 61 checks passed
@freddyDOTCMS freddyDOTCMS deleted the issue-36151-Implement-new-site/folder-field-component-Edit-Contentlet branch June 24, 2026 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Documentation PR changes documentation files

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Implement new site/folder field component (Edit Contentlet)

2 participants