docs(spec): job queue domain queues bypass their dedicated endpoint (#37883)

[ihoffmann-dot]

Proposed Changes

• PR 1 of 2 (spec only) for [DEFECT] /api/v1/jobs/folderBulkDuplicate skips the 50-folder limit that Content Drive's bulk duplicate enforces #37883. Adds specs/37883-job-queue-domain-queues-bypass/spec.md; no code. Implementation follows in PR 2 after this spec is approved.
• The spec widens the issue: POST /api/v1/jobs/{queueName} (and /upload) skips the validation and authorization of five queues that have a dedicated endpoint: folderBulkDuplicate, folderBulkDelete, assetBulkUpload, maintenanceFixAssets, maintenanceCleanAssets.
• Decisions to review:
  • Option B: the generic endpoint refuses queues that have a dedicated endpoint (a Validator per processor cannot carry delete's overlap lock, upload's multipart ceilings or the maintenance permission and cluster lock).
  • The mark is an attribute on @Queue holding the dedicated endpoint's path.
  • The refusal is 403, with a message naming the dedicated endpoint.
  • GET /api/v1/jobs/queues keeps listing marked queues.
  • One PR covers all five queues.

Checklist

• Tests (none yet: spec only; the spec's AC-006 makes each reproduction a failing test first)
• Translations (n/a)
• Security Implications Contemplated: the maintenance queues look like a privilege escalation (a back-end user without the Maintenance portlet could start clean-assets). This comes from reading the code on main and is not reproduced yet; reviewers should weigh how the team wants to handle it (see the spec's last open question).

Additional Info

Open question for reviewers: whether the maintenance finding should follow the security process before this spec is public.

Refs #37883

🤖 Generated with Claude Code

[fabrizzio-dotCMS]

Spec review: requesting changes

The diagnosis is right: five queues accept jobs through POST /api/v1/jobs/{queueName} without going through their dedicated endpoint's checks. My concern is with the fix. It makes two entry models permanent without discussing it, and the protection depends on every developer remembering to mark their queue. I'd like the framework itself to require each queue to declare its entry point.

1. Every queue declares its entry point, and the framework enforces it

The generic endpoint was designed as the single entry point: the queue validates itself and the generic endpoint feeds it. The new features (bulk folder operations, bulk upload, maintenance) put validation in an endpoint of their own instead. The spec accepts that split and makes it permanent with an optional mark on @Queue that defaults to open. If someone adds a new queue with a dedicated endpoint and forgets the mark, that queue stays open.

Proposal:

• The dedicated endpoint declares which queue it feeds, with an annotation on the REST method (e.g. @JobQueueEntryPoint("folderBulkDuplicate")). Queues fed by the generic endpoint declare that on @Queue.
• At startup the framework scans these declarations with Jandex. A core queue that declares no entry point, or more than one, fails startup.
• The generic endpoint accepts only queues that declare it as their entry. Every other queue gets a 403. The path in the message is taken from the annotated method's @Path, not from a hand-copied string, so it can't go stale.
• Plugins: a plugin queue that declares nothing is treated as generic-entry and logs a warning, so existing plugins don't break.

The rationale for rejecting Validator also needs two corrections:

• A Validator can check the maintenance permission. The generic path already adds userId to the parameters (JobQueueHelper.java:116), after the client's parameters, so the client can't spoof it.
• "Leaves future queues open by default" applies equally to an optional @Queue mark.

Two reasons do hold. The folder-delete overlap lock and the maintenance cluster singleton must be held while the job is created. The upload ceilings apply while the multipart is being read. Neither fits in a separate validate() call that runs first. Please also state that the mark guards the REST entry, not Java callers of createJob.

2. importContentlets and bulkRefreshContentlets don't change

Both already implement Validator and don't have the hole. They declare the generic endpoint as their entry and keep answering as they do today. There's no need to turn their 200 into a 403.

3. Scope the security impact, and reproduce it now

The privilege escalation is limited to the two maintenance queues. CleanAssetsJobProcessor does all its work as the system user (resolveSystemUser, line 103). The only barrier is requiredPortlet(MAINTENANCE) in MaintenanceResource, which the generic endpoint doesn't apply. The folder queues run as the submitting user (FolderBulkDeleteProcessor, lines 400 and 622), so what they skip is the load limit and the concurrency guard, not permissions.

Impact is medium. Nobody can choose what gets deleted, but any back-end user can start a clean-up whenever they want, and several at once. That can delete the binary of content that's being saved at that moment, and it loads shared storage.

dotCMS/core is public, so the open question about handling this privately before the spec goes public no longer applies. Please run the AC-006 reproduction for the maintenance case now, with a user who doesn't have the portlet, and set the issue's severity based on the result.

4. AC-003: scope "leaves nothing behind" to what dotCMS creates

A refused submission on the generic endpoint should create nothing dotCMS owns: no job row, no tempFileAPI file, no lock. Putting the check before helper.createJob is enough to guarantee that, and a plain test can prove it without a large upload.

Please don't promise more than that for /upload. The multipart body is spooled before the resource method runs, by Jersey into java.io.tmpdir and, for bodies over 50 MB, into a temp file under the asset temp path. Those spools are collected by BinaryCleanupJob (files older than 3h, during its default midnight cron), so they don't accumulate. Deleting them immediately on refusal would be an improvement to shared upload handling, not part of this fix.

[fabrizzio-dotCMS]

Requesting changes; details in #37906 (comment)

[fabrizzio-dotCMS]

Duplicate of the review above.

[claude]

Claude finished @ihoffmann-dot's task in 1m 54s —— View job

---

Spec review (PR 1 of 2, spec only)

I verified every code reference the spec makes against origin/main, and checked whether this revision answers @fabrizzio-dotCMS's four requested changes.

Code references — all accurate

• JobQueueManagerAPIImpl.createJob validates only when the processor implements Validator — confirmed at JobQueueManagerAPIImpl.java:306.
• userId is added to the params after the client's — confirmed at JobQueueResource's JobQueueHelper.java:116 (parameters.put("userId", user.getUserId())), so the spec is right that a Validator could not be spoofed on this point.
• The maintenance barrier is requireAdmin(true) and requiredPortlet(MAINTENANCE) — confirmed in MaintenanceResource.assertBackendUser (MaintenanceResource.java:1697-1705). The spec is more precise than the review thread here (admin and portlet, not portlet alone) and that precision is correct.
• Both generic creation paths require only requiredBackendUser(true) — confirmed at JobQueueResource.java:124 and :215.
• Dedicated paths exist as written: _fix/_clean at MaintenanceResource.java:788,966; _bulkduplicate at FolderBulkDuplicateResource.java:66; _bulkdelete at FolderBulkDeleteResource.java:60.

Review points addressed

1. Framework enforces a declared entry point — adopted in full (lines 83–90): entry-point annotation on the REST method, Jandex scan, startup failure for a core queue with zero or >1 declarations, message path built from the annotated @Path, plugin-without-declaration warns. The two valid reasons for rejecting Option A (locks/singleton held across creation, upload ceilings during the multipart read) are carried over, and the two incorrect ones are dropped. ✅
2. importContentlets / bulkRefreshContentlets unchanged — stated at lines 36, 104, 118 (keep their 200). ✅
3. Scope the security impact and reproduce now — severity section rewritten (line 38) and AC-006 run on main (confirmed in the thread). ✅
4. AC-003 scoped to what dotCMS owns — line 136 limits the guarantee to no job row / no TempFileAPI file / no lock, with multipart spools explicitly out of scope. ✅

Notes for the plan (non-blocking)

• 🟡 spec.md:100-101 — the declarations table gives concrete paths for duplicate and maintenance but leaves folderBulkDelete ("its bulk-delete endpoint") and assetBulkUpload ("its bulk-upload endpoint") vague, even though _bulkdelete is in the repo today (FolderBulkDeleteResource.java:60). Since AC-005b asserts refusal messages against the real @Path, pin all five paths down so the table and the tests can't drift. Fix this →
• 🟡 spec.md:151 — the single-entry assumption needs to allow for the fact that _fix and _clean each have both a POST (create) and a GET (status) on the same path (MaintenanceResource.java:788 and :824; :966 and :1002). State that the entry-point annotation marks only the creation method, so the "exactly one entry point" startup check doesn't miscount a read method on the same path.
• 🟡 spec.md:62 — the Reproduction's Actual Behavior asserts folderBulkDuplicate ends SUCCESS with processed: 51 as established fact, but the AC-006 run reported in the thread only confirmed "not refused" and explicitly did not check that a jobId/row exists or that the job ran. If the SUCCESS/processed: 51 claim comes from the original issue [DEFECT] /api/v1/jobs/folderBulkDuplicate skips the 50-folder limit that Content Drive's bulk duplicate enforces #37883 reproduction rather than this run, say so; otherwise soften it to match what AC-006 actually verified.

The diagnosis and the reworked fix are sound and the spec is internally consistent with the code it cites. The three notes above are clarity items for /speckit-plan, not blockers for approving the spec.
· branch issue-37883-job-queue-domain-queues-bypass

[fabrizzio-dotCMS]

Thanks, Ivan — the rework covers my review. Approving; one non-blocking correction to fold in before merge:

The maintenance endpoints check admin + portlet, not just the portlet. MaintenanceResource.assertBackendUser requires requireAdmin(true) and requiredPortlet(Portlet.MAINTENANCE) (L1700, L1703), so through the generic endpoint any back-end user, admin or not, can start these jobs. My review understated this too. Please name both checks in the Severity paragraph, and in AC-002/AC-006 make the user a non-admin back-end user without the Maintenance portlet. The fix itself doesn't change.

Nit: in the declarations table, list POST /api/v1/maintenance/assets/_fix and /assets/_clean separately.

On the open question: 👍 to deciding core vs plugin by the Jandex index rather than the package name.

[ihoffmann-dot]

AC-006 reproduction

Ran the reproduction on unmodified main (origin/main + only this spec). The bypass is confirmed on all five queues.

Setup: integration test calling JobQueueResource.createJob (the POST /api/v1/jobs/{queueName} JSON path) as a freshly created user with the back-end role only: no admin, no Maintenance portlet. Each case asserts the behavior the spec requires (a refusal), so it fails today.

Queue	Input	Result on main
folderBulkDuplicate	51 paths	not refused
folderBulkDelete	51 paths	not refused
assetBulkUpload	no files	not refused
maintenanceFixAssets	{}	not refused
maintenanceCleanAssets	{}	not refused

For maintenance this confirms the privilege escalation: a back-end user without the portlet can create both jobs, which MaintenanceResource would refuse.

What this does and does not show

• "Not refused" means the call returned without an exception. The test does not yet assert that a jobId or job row exists.
• I did not check whether the maintenance jobs ran, or what they did.
• Only the JSON path was exercised; the /upload path is not covered yet.

The test is not in this PR (it is spec only). It will go into PR 2 together with the fix, registered in a Junit5Suite*. Next: severity of the issue can be set from the maintenance result.

[ihoffmann-dot]

@fabrizzio-dotCMS heads-up on one scope change from the approved spec. I am moving to another project and the team will continue; the details are in the handover comment on #37883: #37883 (comment)

The fix is on branch issue-37883-job-queue-entry-points (no PR yet): the generic endpoint now refuses the five dedicated-entry queues with 403 naming the real route, every core queue declares its entry point, and a build-time test asserts exactly one declaration per queue. Tests are green, including the regression ITs of the dedicated endpoints.

What I did not implement: the check in JobQueueManagerAPIImpl.start() that refuses to start the queue manager when a core queue does not have exactly one declaration. Today only the build-time coverage test guards it. So the spec's "fails startup" is not true at runtime yet. Whoever picks this up can either implement it (and first check what InitServlet.init() does when start() throws, InitServlet.java:233) or change the spec wording to "the build fails".

Also pending: the short doc for new queues and the ADR proposed in the plan.