Skip to content

Add report issue workflow to backend and toolbar UI#35717

Merged
fmontes merged 24 commits into
mainfrom
fmontes/report-bug-button
May 19, 2026
Merged

Add report issue workflow to backend and toolbar UI#35717
fmontes merged 24 commits into
mainfrom
fmontes/report-bug-button

Conversation

@fmontes
Copy link
Copy Markdown
Member

@fmontes fmontes commented May 15, 2026

report-issue-2.mp4

Proposed Changes

  • add a new /api/v1/report-issue backend endpoint that validates report submissions, forwards them to the workflow action, and handles proxy and upstream failures explicitly
  • add the toolbar user-menu entry and report issue dialog in dotCMS UI, including multipart submission, client metadata capture, PrimeNG file upload, and inline validation/error handling
  • add tests for the backend resource and UI service/component flows, and add the required UI translations and OpenAPI documentation

Checklist

  • Tests
  • Translations
  • Security Implications Contemplated (report submission stays server-side and relies on the configured user proxy instead of exposing a token in the UI)

Additional Info

This diff includes the follow-up typing cleanup for the UI report-issue service response and the signal-based/report-component refactors that are already on this branch.
It also adds REPORT_ISSUE_INCLUDE_USER_PII; set it to false to force anonymous reports and show the UI hint, or true to allow user details in report metadata.

We use a plugin for the cross-env auth: https://github.com/dotcms-community/com.dotcms.userproxy/

Screenshots

Original Updated
N/A N/A

fmontes and others added 4 commits May 14, 2026 18:25
Convert @Input/@output to model()/output(), replace visible setter
side-effect with effect(), and promote screenshotFile to a signal for
OnPush correctness.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move dot-report-issue out of dot-toolbar/components since it is no
longer toolbar-specific. Update relative imports accordingly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

❌ Issue Linking Required

This PR could not be linked to an issue. All PRs must be linked to an issue for tracking purposes.

How to fix this:

Option 1: Add keyword to PR body (Recommended - auto-removes this comment)
Edit this PR description and add one of these lines:

  • This PR fixes #123 or Fixes: #123

  • This PR closes #123 or Closes: #123

  • This PR resolves #123 or Resolves: #123

  • Other supported keywords: fix, fixed, close, closed, resolve, resolved
    Option 2: Link via GitHub UI (Note: won't clear the failed check)

  1. Go to the PR → Development section (right sidebar)

  2. Click "Link issue" and select an existing issue

  3. Push a new commit or re-run the workflow to clear the failed check
    Option 3: Use branch naming
    Create a new branch with one of these patterns:

  • 123-feature-description (number at start)

  • issue-123-feature-description (issue-number at start)

  • feature-issue-123 (issue-number anywhere)

Why is this required?

Issue linking ensures proper tracking, documentation, and helps maintain project history. It connects your code changes to the problem they solve.---

This comment was automatically generated by the issue linking workflow

@github-actions github-actions Bot added Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code labels May 15, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 15, 2026

Claude finished @fmontes's task in 4m 32s —— View job


PR Review — Report Issue workflow

Done reviewing. The PR is well-structured, well-tested, and the two Copilot rounds already caught most of the obvious issues. A few things still look worth a second look — flagging the higher-impact items first.


1. remoteAddress is leaked even when the user is "anonymous" — privacy hole

ReportIssueResource.buildMetadata() (lines 444–467) unconditionally puts remoteAddress, serverName, userAgent, and referer in the metadata, regardless of the anonymous flag or the REPORT_ISSUE_INCLUDE_USER_PII operator opt-out. The "Send anonymously" toggle only gates the user block (userId / email / fullName).

In practice this means:

  • User checks "Send anonymously" → upstream still receives their IP, browser, referer, full request URL.
  • Operator sets REPORT_ISSUE_INCLUDE_USER_PII=false for a data-residency / GDPR posture → same; client IP still ships off-instance.

User IP is identifying data, and "anonymous" sets metadata.anonymous = true next to a remoteAddress, which is contradictory. Either drop remoteAddress / referer / serverName when !includeUserIdentity, or rename the toggle to something narrower (e.g. "Don't attach my dotCMS user account"). The current naming overpromises. Fix this →

2. No size cap on the metadata form field — authenticated DoS vector

Screenshots get a 10MB cap via MAX_SCREENSHOT_BYTES_PROPERTY, but the metadata JSON field is passed straight to objectMapper.readValue(metadataField.getValue(), ...) (line 213-215). FormDataBodyPart.getValue() returns the whole string in memory, then Jackson parses it into a Map<String, Object> — also in memory. An authenticated backend user can POST a multi-hundred-MB metadata field. Same is true for the description field (no length check). Suggest a hard cap (e.g. a few KB for metadata, a few KB for description), and reject above it with the same ReportIssueValidationException path. Fix this →

3. Hard-coded default upstream URL → on-by-default cross-host forwarding

DEFAULT_WORKFLOW_URL = "https://corpsites-headless.dotcms.cloud/..." (line 63-64) makes the feature live on every install that doesn't explicitly opt out. Combined with #1, an on-prem customer who has never heard of this feature can have user IPs forwarded to a dotCMS Cloud host. Copilot flagged this and the author acknowledged it as a follow-up. I'd still push to invert the default before merge: empty default → 502 with a clear "feature not configured" error. Less risk than the current "on by default for everyone, opt-out via env var."

4. DotReportIssueService.reportIssue() return type doesn't match what the backend emits

return this.http
    .post<DotCMSResponse<DotReportIssueContentlet>>('/api/v1/report-issue', formData)
    .pipe(map((response) => response.entity));

The backend (mapUpstreamResponse) passes the upstream workflow-fire response body through unchanged. The upstream returns a workflow action result, not a DotCMSResponse<DotReportIssueContentlet> with .entity. So response.entity is most likely undefined at runtime, and the typed Observable<DotReportIssueContentlet> is misleading. The component only checks for completion, so this is harmless today, but if anyone consumes the value later they'll get undefined. Either type the response honestly (unknown / a workflow-fire view) or normalize the upstream body server-side into a ResponseEntityView.

5. OpenAPI 200 schema is empty / generic

"200":
  content:
    application/json: {}
  description: Issue reported

Per dotCMS/src/main/java/com/dotcms/rest/CLAUDE.md, the 200 response should reference a specific view class, and 400/502 currently use generic ResponseEntityView (also flagged by the AI bot). Worth introducing a small ReportIssueResponseView for the success body — even if the upstream shape is opaque, a type=object, description=... schema would be more useful than nothing.

6. [visible] is one-way on the parent

In dot-toolbar-user.component.html:

<dot-report-issue (shutdown)="closeReportIssue()" [visible]="$showReportIssue()" />

The dialog declares visible = model<boolean>(false) and requestClose() does this.visible.set(false). With a one-way [visible]="..." binding the parent doesn't observe the child writing back to the model; close is only propagated via shutdown. It works today because PrimeNG dialog's onHide fires synchronously and the parent updates the signal immediately, but the pattern is fragile — if anything ever runs between visible.set(false) and shutdown.emit(), an intermediate CD pass can write true back through the input. Cleanest fix: [(visible)]="$showReportIssue" (pass the signal, not ()).


Smaller things / nits

  • mapUpstreamResponse passes upstream 4xx (e.g. 415) through as-is (line 516-521), but the endpoint's OpenAPI only documents 200/400/502. The frontend tests already exercise a 415 path, so the behavior is "intentional, undocumented." Either map non-401/403 4xx to a normalized 400/502, or document the passthrough in the OpenAPI annotations.
  • extractVersion return value is unused in parseUserAgent (line 399–402) — only UtilMethods.isSet(version) matters; the match is enough. Could be inlined as a contains(token) for clarity.
  • No test coverage for image/webp in ALLOWED_SCREENSHOT_TYPES — only PNG is tested. Trivial to add.
  • buildJsonRequest declares throws IOException but only writeValueAsString can throw — fine, just noting it propagates correctly.
  • title.length() <= 120 ? ... : title.substring(0, 120) — fine, but if title.length() is exactly 120 it returns the unbounded string; the 120 boundary is correct, just confirming on purpose.
  • escapeHeaderValue is redundant after FileUtil.sanitizeFileName (which already strips ", \, and all control chars 0–31). Defensive duplication is fine.
  • hasSubmitted is set inside save() even when the form is invalid (which is what surfaces error messages) — works but means a user can't get back to a "pristine" UI without closing/reopening the dialog. Probably intentional.
  • @SuppressWarnings("unchecked") on clientMetadata cast — fine, but consider replacing the cast with objectMapper.convertValue(...) if you ever want stronger typing.
  • HttpClient is reused per HttpReportIssueForwarder instance but a fresh ReportIssueResource is built per request (JAX-RS default lifecycle), so a fresh HttpClient is created per request. Not catastrophic — JDK HttpClient is heavy. Worth caching a singleton (or making ReportIssueResource @Singleton/@Provider).
  • No CSRF mentionWebResource.InitBuilder with requiredBackendUser should provide the standard dotCMS protection; just confirming this is the expected pattern.

Tests

The backend test coverage is solid (anonymous flag, operator opt-out, screenshot type / size, upstream 401/403/network failure, multipart body construction, auth header absence, stream close). Two gaps worth adding:

  • image/webp happy path
  • Oversized metadata field once Test Branch and Commit #2 is addressed
  • Title truncation at the 120-char boundary

Frontend tests look thorough. One small thing: the closeReportIssue flow on the parent DotToolbarUserComponent (the (shutdown)$showReportIssue.set(false) path) isn't covered in dot-toolbar-user.component.spec.ts from what I read; worth a sanity test.


Summary

Approve the structure and most of the implementation — the recent commits already addressed Copilot's repeat findings. Blocking-ish, in order: (1) anonymous-toggle leaks IP (it's a real privacy semantics bug, not a nit), (2) unbounded metadata field, and (3) the on-by-default external default URL. The rest are clean-up that can ship as follow-ups.
· fmontes/report-bug-button

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an end-to-end “Report an Issue” workflow for the dotCMS Admin UI by introducing a new backend proxy endpoint that forwards issue reports (optionally with a screenshot) to an upstream workflow action, and wiring a new toolbar user-menu entry + dialog that submits multipart data and surfaces validation/errors.

Changes:

  • Backend: Introduces POST /api/v1/report-issue JAX-RS resource, request validation, upstream forwarding (JSON or multipart), and response mapping; adds unit tests.
  • Frontend: Adds “Report an Issue” menu item and a new dialog component to collect description + optional screenshot and submit via a new API service.
  • Docs/i18n: Adds OpenAPI path entry and new UI translation keys.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
dotCMS/src/main/java/com/dotcms/rest/api/v1/reportissue/ReportIssueResource.java New REST endpoint + upstream forwarder and payload/validation logic for report submissions.
dotCMS/src/test/java/com/dotcms/rest/api/v1/reportissue/ReportIssueResourceTest.java Unit tests covering validation, forwarding behavior, and authorization header handling.
dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml Documents the new /v1/report-issue endpoint in OpenAPI.
dotCMS/src/main/webapp/WEB-INF/messages/Language.properties Adds i18n strings for the new “Report an Issue” UI flow.
core-web/apps/dotcms-ui/src/app/view/components/dot-toolbar/components/dot-toolbar-user/store/dot-toolbar-user.store.ts Adds state + menu action to open the report-issue dialog.
core-web/apps/dotcms-ui/src/app/view/components/dot-toolbar/components/dot-toolbar-user/store/dot-toolbar-user.store.spec.ts Tests new menu item and showReportIssue state updates.
core-web/apps/dotcms-ui/src/app/view/components/dot-toolbar/components/dot-toolbar-user/dot-toolbar-user.component.ts Includes the new report-issue dialog component in toolbar user component imports.
core-web/apps/dotcms-ui/src/app/view/components/dot-toolbar/components/dot-toolbar-user/dot-toolbar-user.component.html Renders the report-issue dialog when store state indicates it should be visible.
core-web/apps/dotcms-ui/src/app/view/components/dot-toolbar/components/dot-toolbar-user/dot-toolbar-user.component.spec.ts Updates test setup to provide services required by the newly imported dialog component.
core-web/apps/dotcms-ui/src/app/view/components/dot-report-issue/dot-report-issue.component.ts New dialog component with reactive form validation, screenshot handling, and submission.
core-web/apps/dotcms-ui/src/app/view/components/dot-report-issue/dot-report-issue.component.html Dialog UI markup, inputs, file upload, and submit/cancel actions.
core-web/apps/dotcms-ui/src/app/view/components/dot-report-issue/dot-report-issue.component.spec.ts Component tests for validation, submission flow, and error handling.
core-web/apps/dotcms-ui/src/app/providers.ts Registers the new report-issue API service in app-wide providers.
core-web/apps/dotcms-ui/src/app/api/services/dot-report-issue.service.ts New Angular service to submit multipart form data to /api/v1/report-issue.
core-web/apps/dotcms-ui/src/app/api/services/dot-report-issue.service.spec.ts Service tests ensuring multipart fields are sent (with/without screenshot).
Comments suppressed due to low confidence (4)

dotCMS/src/main/java/com/dotcms/rest/api/v1/reportissue/ReportIssueResource.java:449

  • Client-supplied metadata is merged into the server-built metadata via metadata.putAll(clientMetadata), which allows a malicious client to override trusted server fields (e.g., submittedAt, user, remoteAddress, serverName) and spoof report context. Consider either nesting client metadata under a dedicated key (e.g., metadata.client) or whitelisting/merging in a way that prevents overriding server-owned keys.
        if (user != null) {
            final Map<String, Object> userMetadata = new LinkedHashMap<>();
            userMetadata.put("userId", Try.of(user::getUserId).getOrNull());
            userMetadata.put("email", Try.of(user::getEmailAddress).getOrNull());
            userMetadata.put("fullName", Try.of(user::getFullName).getOrNull());
            metadata.put("user", userMetadata);
        }

        if (clientMetadata != null && !clientMetadata.isEmpty()) {
            metadata.putAll(clientMetadata);
        }

dotCMS/src/main/java/com/dotcms/rest/api/v1/reportissue/ReportIssueResource.java:274

  • readBytesWithLimit reads the screenshot InputStream but never closes it. Other multipart handlers in this codebase use try-with-resources around part.getEntityAs(InputStream.class) to ensure streams are closed; consider doing the same here to avoid leaking request resources under load.
    private static byte[] readBytesWithLimit(final InputStream inputStream, final long maxBytes)
            throws IOException {
        if (inputStream == null) {
            throw new ReportIssueValidationException("Screenshot is invalid.");
        }

        final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
        final byte[] buffer = new byte[8192];
        long totalBytes = 0L;
        int bytesRead;

        while ((bytesRead = inputStream.read(buffer)) != -1) {
            totalBytes += bytesRead;
            if (totalBytes > maxBytes) {
                throw new ReportIssueValidationException("Screenshot exceeds the maximum allowed size.");
            }
            outputStream.write(buffer, 0, bytesRead);
        }

        return outputStream.toByteArray();

dotCMS/src/main/java/com/dotcms/rest/api/v1/reportissue/ReportIssueResource.java:118

  • The OpenAPI annotations for 400/502 use the generic ResponseEntityView schema, and the 200 response has no schema. In this codebase, other endpoints typically reference a specific response view type (e.g., TempFilesView) to keep the contract explicit; consider introducing a dedicated response view for this endpoint (success + error) and referencing it in @Schema to avoid an untyped/ambiguous API contract.
    @Operation(
            operationId = "reportIssue",
            summary = "Report a dotCMS UI issue",
            description = "Creates a Bug contentlet in the configured upstream reporting dotCMS instance.",
            tags = {"Workflow"},
            responses = {
                    @ApiResponse(responseCode = "200", description = "Issue reported",
                            content = @Content(mediaType = "application/json")),
                    @ApiResponse(responseCode = "400", description = "Invalid report payload",
                            content = @Content(mediaType = "application/json",
                                    schema = @Schema(implementation = ResponseEntityView.class))),
                    @ApiResponse(responseCode = "502", description = "Reporting service unavailable or unauthorized",
                            content = @Content(mediaType = "application/json",
                                    schema = @Schema(implementation = ResponseEntityView.class)))

dotCMS/src/main/java/com/dotcms/rest/api/v1/reportissue/ReportIssueResource.java:485

  • mapUpstreamResponse can return upstream 4xx responses directly (e.g., 415) when the upstream includes a body, but the endpoint’s OpenAPI annotations (and openapi.yaml entry) only document 200/400/502. Consider either normalizing these upstream errors into the documented 502/400 responses, or expanding the OpenAPI responses to include the possible passthrough status codes so clients have an accurate contract.
    private Response mapUpstreamResponse(final ReportIssueForwardResponse upstreamResponse) {
        final int statusCode = upstreamResponse.statusCode();

        if (statusCode == Response.Status.UNAUTHORIZED.getStatusCode()
                || statusCode == Response.Status.FORBIDDEN.getStatusCode()) {
            return error(Response.Status.BAD_GATEWAY, ERROR_PROXY_NOT_AUTHORIZED,
                    "Report issue service is not authorized. Check the User Proxy plugin configuration.");
        }

        if (statusCode >= 200 && statusCode < 300) {
            return Response.status(statusCode)
                    .entity(upstreamResponse.body())
                    .type(upstreamResponse.contentType().orElse(MediaType.APPLICATION_JSON))
                    .build();
        }

        if (statusCode >= 400 && statusCode < 500 && UtilMethods.isSet(upstreamResponse.body())) {
            return Response.status(statusCode)
                    .entity(upstreamResponse.body())
                    .type(upstreamResponse.contentType().orElse(MediaType.APPLICATION_JSON))
                    .build();
        }

fmontes and others added 6 commits May 17, 2026 20:57
- Backend: nest client-supplied metadata under "client" key so server
  fields cannot be spoofed; update title/browser derivation and tests.
- Backend: document why the upstream multipart request uses PUT while
  the JSON branch uses POST.
- Frontend: convert getScreenshotErrorMessage to a computed signal
  driven by form.statusChanges via toSignal.
- Frontend: drop isClosing/queueMicrotask workaround now that handleClose
  flips the visible model directly.
- Frontend: remove redundant take(1) operators on one-shot HttpClient
  observables.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address review feedback on external PII forwarding:

- Frontend: add "Send anonymously" checkbox in the dialog; when checked,
  the request is submitted with anonymous=true.
- Backend: when anonymous=true, strip the user.* identity block from
  the forwarded metadata. The metadata.anonymous flag is always set so
  the upstream can audit whether identity was sent.
- Backend: add REPORT_ISSUE_INCLUDE_USER_PII config flag (default true).
  When operators set it to false, user identity is always stripped
  regardless of the client checkbox.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rename INCLUDE_USER_PII_PROPERTY to REPORT_ISSUE_INCLUDE_USER_PII_PROPERTY
(and the matching default) so the Java constant name reflects the
report-issue scope. The Config property key is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When the operator sets REPORT_ISSUE_INCLUDE_USER_PII=false:

- Expose the property via the existing /api/v1/configuration/config
  endpoint by adding it to the WHITE_LIST.
- Frontend reads the value via DotPropertiesService.getKey with the
  boolean: prefix.
- The "Send anonymously" checkbox is forced checked and disabled, and
  an inline hint explains that anonymous submission is enforced by the
  administrator.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The submit button is rendered inside PrimeNG's footer ng-template,
outside the <form>, so type="submit" never triggered (ngSubmit). Switch
to type="button" so the markup matches what the click handler actually
does.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.

Comment thread dotCMS/src/main/java/com/dotcms/rest/api/v1/reportissue/ReportIssueResource.java Outdated
@fmontes fmontes added this pull request to the merge queue May 19, 2026
Merged via the queue into main with commit 8197c07 May 19, 2026
52 checks passed
@fmontes fmontes deleted the fmontes/report-bug-button branch May 19, 2026 17:32
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 : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Add Report an Issue workflow to dotCMS admin toolbar

3 participants