Skip to content

Address add-filter toast XSS path by sanitizing dynamic notification content - #5718

Closed
andreasfritz with Copilot wants to merge 5 commits into
masterfrom
copilot/fix-code-as-per-review-comment
Closed

andreasfritz with Copilot wants to merge 5 commits into
masterfrom
copilot/fix-code-as-per-review-comment

Conversation

Copilot AI commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

The review comment identified an unsanitized HTML path in the applications add-filter flow: the success toast interpolated applicationName || instanceId directly into an HTML message. This leaves an XSS gap even though related action handlers already sanitize similar notification content.

  • Scope from review comment

    • Apply the fix only to the add-filter success notification path in views/applications/index.vue.
    • Add a regression test that covers malicious filter names/instance IDs in the add-filter flow.
  • Code change

    • Sanitize the constructed toast message before passing it to notificationCenter.success, matching the existing sanitization approach used in other application/instance action notifications.
  • Regression coverage

    • Add/extend applications view tests to assert that injected markup (e.g. <img ... onerror=...> / <script>) is stripped in the add-filter success toast path.
notificationCenter.success(
  sanitizeHtml(
    `${t('applications.notifications_suppressed_for', {
      name: notificationFilter.applicationName || notificationFilter.instanceId,
    })} <strong>${notificationFilter.expiry.fromNow(true)}</strong>.`,
  ),
);

SteKoe and others added 5 commits September 25, 2026 08:10
String-valued health/info detail values from a monitored instance's
actuator response were inserted via v-html without sanitization,
while the sibling object-valued path already sanitized its output.
This allowed a malicious or compromised instance to store markup
that executes as script in an administrator's browser session
(stored XSS, CWE-79).

Apply the same sanitizeHtml() call used by sba-formatted-obj.vue
before autolinking the value.
A follow-up scan of every v-html usage in spring-boot-admin-server-ui
found three more sinks rendering untrusted, remote-instance-controlled
data without sanitization or escaping:

- env/refresh.vue: property names from an instance's /actuator/refresh
  response were concatenated into a raw HTML string and rendered via
  v-html. Replaced with a v-for/v-text list so Vue escapes each entry.

- NotificationFilterSettings.vue: the registered application/instance
  name (attacker-controlled at registration time) was interpolated
  into an i18n string containing literal <code> markup and rendered
  via v-html; vue-i18n does not escape interpolated params. Switched
  to <i18n-t> with a scoped slot so the name is rendered as text.

- m-bean-operation.vue: a JMX MBean operation name (controlled by the
  monitored JVM, e.g. a compromised instance) was interpolated the
  same way. Fixed identically with <i18n-t>.

Also switched sba-alert.vue's error message from v-html to v-text as
defense in depth, since Jolokia-derived error strings can originate
from a monitored instance and no legitimate use case needs HTML
rendering there.

Added regression tests asserting injected markup/scripts are never
parsed as HTML for the health-details, m-bean-operation, and
NotificationFilterSettings components.
Previous commit switched sba-alert.vue from v-html to v-text as a
blunt defense-in-depth measure, but error messages sometimes contain
intentional HTML (e.g. <strong>) that callers want rendered.

Keep v-html but sanitize the message through sanitizeHtml() first,
matching the pattern already used by sba-formatted-obj.vue and
health-details.vue. Safe markup renders; script tags and event
handler attributes are stripped.

Adds a dedicated spec file covering plain text, safe HTML, and
malicious payloads for both string and Error inputs.
A deeper audit of innerHTML/v-html sinks fed by application/instance
names uncovered a previously-missed vulnerable path: ActionHandler.ts
interpolates the attacker-controllable application name / instance id
into i18n strings (some containing literal <code> markup) and passes
the result to $sbaModal.confirm() and notificationCenter.success/
error(). Both render their message via innerHTML - functionally
identical to v-html - with no sanitization, for every restart/
shutdown/unregister confirmation dialog and result toast.

Wrap every such call in sanitizeHtml(), consistent with the pattern
already used in health-details.vue and sba-alert.vue. Restore the
<code>{name}</code> markup in the i18n files (previously stripped as
a workaround) now that the value is sanitized at the render boundary
instead of avoided via <i18n-t> + v-text.

Also revert NotificationFilterSettings.vue and m-bean-operation.vue
from the <i18n-t> + v-text workaround back to v-html + sanitizeHtml(),
so all v-html sinks in the codebase follow one consistent mitigation
pattern.

Adds ActionHandler.spec.ts (previously untested) covering all
shutdown/restart/unregister paths for both handlers, and updates the
NotificationFilterSettings/m-bean-operation specs to assert dangerous
markup is stripped while safe formatting tags are preserved.
Copilot AI changed the title [WIP] Fix code based on review comment Address add-filter toast XSS path by sanitizing dynamic notification content Sep 25, 2026
Base automatically changed from fix/health-details-xss to master September 25, 2026 07:04
Copilot AI requested a review from andreasfritz September 25, 2026 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants