Skip to content

fix(job-queue): generic job endpoint refuses queues that have a dedicated endpoint (#37883) - #37946

Open
ihoffmann-dot wants to merge 2 commits into
issue-37883-job-queue-domain-queues-bypassfrom
issue-37883-job-queue-entry-points
Open

ihoffmann-dot wants to merge 2 commits into
issue-37883-job-queue-domain-queues-bypassfrom
issue-37883-job-queue-entry-points

Conversation

@ihoffmann-dot

@ihoffmann-dot ihoffmann-dot commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

Proposed Changes

Checklist

  • Tests: JobQueueEntryPointsTest (7), JobQueueEntryPointsCoverageTest (2, one declaration per core queue), JobQueueDomainQueuesBypassIntegrationTest (8, registered in Junit5Suite1). Regression green: FolderBulkDuplicateResourceIT, FolderBulkDeleteResourceIT, BulkUploadResourceIT, MaintenanceResourceIntegrationTest, JobQueueHelperIntegrationTest.
  • Translations (n/a)
  • Security Implications Contemplated: closes a missing authorization on the maintenance queues. The dedicated endpoints require admin and the Maintenance portlet; before this change any back-end user could create those jobs through the generic endpoint (reproduced on main).

Additional Info

Rollback: REST contract change, 200 to 403 on five queues, no response shape added. Expected rollback safe (M-3 of docs/core/ROLLBACK_UNSAFE_CATEGORIES.md); @Queue gains a member with a default (M-4, compatible).

Known gaps (details in the handover comment on #37883):

  • The check in JobQueueManagerAPIImpl.start() that refuses to start the queue manager when a core queue does not have exactly one declaration is not implemented; only the build-time coverage test guards it. The spec still says "fails startup", so either implement it (checking first what InitServlet.init() does when start() throws) or align the spec wording.
  • No doc for new queues yet, and the proposed ADR ("every job queue declares its entry point, and the framework enforces it") is not written.
  • /speckit-converge has not been run.

Refs #37883

🤖 Generated with Claude Code

@claude

claude Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @ihoffmann-dot's task in 2m 44s —— View job


Code Review

Reviewed the full diff against origin/issue-37883-job-queue-domain-queues-bypass. The design is clean: entry points are declared on the annotated methods and read from the Jandex index, routes in the 403 message come from the real @Path (verified ForbiddenException → HttpStatusCodeException.getFormattedMessage does String.format, so the %s placeholders resolve), nothing is created on refusal, and the gate is placed after auth but before createJob on both creation methods. Test coverage (unit + coverage guard + 8 integration cases, registered in Junit5Suite1) is thorough.

New Issues

  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueEntryPoints.java:88,110 — the genericCoreQueues field is populated in the constructor but never read. In the index-present branch of resolve() (lines 153–161), a core queue that has @Queue but declares neither genericEntry = true nor a @JobQueueEntryPoint is not in dedicatedRoutes, is in coreQueues (so no warning), and falls through to return EntryPoint.GENERIC — i.e. a misconfigured core queue falls open at runtime. The only thing catching that is the build-time JobQueueEntryPointsCoverageTest; the runtime does not enforce it, which is weaker than the class javadoc's "fails closed" framing and is the same hole as the acknowledged "start() fails startup — not implemented" gap. Either consult genericCoreQueues to fail closed for undeclared core queues, or remove the dead field. Fix this →

  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueEntryPoints.java:127-139 — getInstance() memoizes the first registry for the JVM lifetime. If the first resolve() call ever runs while JandexClassMetadataScanner.getJandexIndex() returns null, the fail-closed (index == null) registry is cached permanently, and every plugin queue that doesn't set genericEntry = true would get 403 for the rest of the process. Assumption: the index could momentarily be null early in startup. What to verify: that the index is always populated before the /api/v1/jobs/{queueName} endpoint can serve a request; if it cannot be guaranteed, avoid caching a null-index instance (only memoize when the index is non-null). If serving is always post-init, this is not reachable — hence Medium.

Notes (non-blocking, already acknowledged in the PR body)

  • The spec's "fails startup when a core queue has no/▸1 declaration" is not implemented — only the build-time coverage test guards it (ties into the first finding).
  • Minor: assertGenericEntry logs a warn (JobQueueHelper.java:66) and HttpStatusCodeException logs another warn when the exception is constructed, so each refusal logs twice. Harmless.
  • No new doc / ADR for the "every queue declares its entry point" rule, and /speckit-converge not yet run — both called out by the author.

No bugs that block merge; the two medium items are worth resolving (the first especially, since the dead field reflects enforcement that isn't actually happening at runtime).

``
· issue-37883-job-queue-entry-points

@ihoffmann-dot
ihoffmann-dot marked this pull request as ready for review October 8, 2026 02:09

This branch has not been deployed

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

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[DEFECT] /api/v1/jobs/folderBulkDuplicate skips the 50-folder limit that Content Drive's bulk duplicate enforces

1 participant