diff --git a/spring-boot-admin-server-ui/src/main/frontend/components/sba-alert.spec.ts b/spring-boot-admin-server-ui/src/main/frontend/components/sba-alert.spec.ts new file mode 100644 index 00000000000..73a33453bfd --- /dev/null +++ b/spring-boot-admin-server-ui/src/main/frontend/components/sba-alert.spec.ts @@ -0,0 +1,67 @@ +/* + * Copyright 2014-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { screen } from '@testing-library/vue'; +import { describe, expect, it } from 'vitest'; + +import SbaAlert from '@/components/sba-alert.vue'; + +import { render } from '@/test-utils'; + +describe('SbaAlert', () => { + it('renders plain-text error messages', async () => { + render(SbaAlert, { + props: { error: 'Something went wrong' }, + }); + + expect(await screen.findByText('Something went wrong')).toBeInTheDocument(); + }); + + it('renders safe HTML markup contained in the error message', async () => { + render(SbaAlert, { + props: { error: 'Request failed: timeout' }, + }); + + const strong = await screen.findByText('timeout'); + expect(strong.tagName).toBe('STRONG'); + }); + + it('strips script tags and event handler attributes from the error message', async () => { + const maliciousMessage = + ''; + + render(SbaAlert, { + props: { error: maliciousMessage }, + }); + + await screen.findByRole('alert'); + + expect(document.querySelectorAll('img').length).toBe(0); + expect(document.querySelectorAll('script').length).toBe(0); + expect((window as any).__xss).toBeUndefined(); + }); + + it('sanitizes Error instance messages the same way', async () => { + render(SbaAlert, { + props: { + error: new Error('boom'), + }, + }); + + expect(await screen.findByText('boom')).toBeInTheDocument(); + expect(document.querySelectorAll('img').length).toBe(0); + expect((window as any).__xss).toBeUndefined(); + }); +}); diff --git a/spring-boot-admin-server-ui/src/main/frontend/components/sba-alert.vue b/spring-boot-admin-server-ui/src/main/frontend/components/sba-alert.vue index 1931df93965..f24fb517e47 100644 --- a/spring-boot-admin-server-ui/src/main/frontend/components/sba-alert.vue +++ b/spring-boot-admin-server-ui/src/main/frontend/components/sba-alert.vue @@ -42,6 +42,8 @@ import { defineComponent } from 'vue'; import FontAwesomeIcon from '@/components/font-awesome-icon'; +import { sanitizeHtml } from '@/utils/sanitizeHtml'; + export const Severity = { ERROR: 'ERROR', WARN: 'WARN', @@ -90,10 +92,10 @@ export default defineComponent({ computed: { message() { if (this.error instanceof Error) { - return this.error.message; + return sanitizeHtml(this.error.message); } if (typeof this.error === 'string') { - return this.error; + return sanitizeHtml(this.error); } return null; diff --git a/spring-boot-admin-server-ui/src/main/frontend/views/applications/ActionHandler.spec.ts b/spring-boot-admin-server-ui/src/main/frontend/views/applications/ActionHandler.spec.ts new file mode 100644 index 00000000000..ccf7c3dc7e1 --- /dev/null +++ b/spring-boot-admin-server-ui/src/main/frontend/views/applications/ActionHandler.spec.ts @@ -0,0 +1,226 @@ +/* + * Copyright 2014-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { describe, expect, it, vi } from 'vitest'; + +import { + ApplicationActionHandler, + InstanceActionHandler, +} from '@/views/applications/ActionHandler'; + +// A minimal translation function that mimics vue-i18n's interpolation: +// it does NOT escape the injected {name}/{error} values, so any HTML +// contained in them ends up verbatim in the returned string - exactly +// like the real i18n messages that still contain literal markup. +const t = (key: string, params: Record = {}) => { + const templates: Record = { + 'applications.actions.unregister': 'Deregister?', + 'applications.actions.shutdown': 'Shutdown?', + 'applications.actions.restart': 'Restart?', + 'applications.unregister': 'Deregister application {name}?', + 'applications.unregister_successful': + 'Successfully deregistered application {name}.', + 'applications.unregister_failed': + 'Deregistration of application {name} failed ({error}).', + 'applications.shutdown': 'Shutdown application {name}?', + 'applications.shutdown_successful': + 'Successfully shutdown application {name}.', + 'applications.shutdown_failed': 'Failed to shutdown application {name}.', + 'applications.restart': 'Restart application {name}?', + 'applications.restarted': + 'Successfully restarted application {name}.', + 'applications.restart_failed': + 'Failed to restart application {name} ({error}).', + 'instances.unregister': 'Deregister instance {name}?', + 'instances.unregister_successful': + 'Successfully deregistered instance {name}.', + 'instances.unregister_failed': + 'Deregistration of instance {name} failed ({error}).', + 'instances.shutdown': 'Shutdown instance {name}?', + 'instances.shutdown_successful': 'Successfully shutdown instance {name}.', + 'instances.shutdown_failed': 'Failed to shutdown instance {name}.', + 'instances.restart': 'Restart instance {name}?', + 'instances.restarted': + 'Successfully restarted instance {name}.', + 'instances.restart_failed': 'Failed to restart instance {name} ({error}).', + }; + let result = templates[key] ?? key; + for (const [param, value] of Object.entries(params)) { + result = result.replaceAll(`{${param}}`, String(value)); + } + return result; +}; + +function createModalStub(confirmed = true) { + return { confirm: vi.fn().mockResolvedValue(confirmed) }; +} + +function createNotificationCenterStub() { + return { success: vi.fn(), error: vi.fn() }; +} + +describe('ApplicationActionHandler', () => { + const maliciousName = ''; + + it('sanitizes the application name in the shutdown confirmation dialog', async () => { + const $sbaModal = createModalStub(false); + const notificationCenter = createNotificationCenterStub(); + const handler = new ApplicationActionHandler( + $sbaModal, + t, + notificationCenter, + ); + + await handler.shutdown({ name: maliciousName } as any); + + const body = $sbaModal.confirm.mock.calls[0][1]; + expect(body).not.toContain(' { + const $sbaModal = createModalStub(false); + const notificationCenter = createNotificationCenterStub(); + const handler = new ApplicationActionHandler( + $sbaModal, + t, + notificationCenter, + ); + + await handler.restart({ name: maliciousName } as any); + + const body = $sbaModal.confirm.mock.calls[0][1]; + expect(body).not.toContain(' { + const $sbaModal = createModalStub(true); + const notificationCenter = createNotificationCenterStub(); + const application = { + name: maliciousName, + unregister: vi.fn().mockResolvedValue(undefined), + }; + const handler = new ApplicationActionHandler( + $sbaModal, + t, + notificationCenter, + ); + + await handler.unregister(application as any); + + const confirmBody = $sbaModal.confirm.mock.calls[0][1]; + expect(confirmBody).not.toContain(' { + const $sbaModal = createModalStub(true); + const notificationCenter = createNotificationCenterStub(); + const application = { + name: maliciousName, + unregister: vi.fn().mockRejectedValue({ response: { status: 500 } }), + }; + const handler = new ApplicationActionHandler( + $sbaModal, + t, + notificationCenter, + ); + + await handler.unregister(application as any); + + const errorMessage = notificationCenter.error.mock.calls[0][0]; + expect(errorMessage).not.toContain(' around the sanitized name', async () => { + const $sbaModal = createModalStub(false); + const notificationCenter = createNotificationCenterStub(); + const handler = new ApplicationActionHandler( + $sbaModal, + t, + notificationCenter, + ); + + await handler.shutdown({ name: 'my-app' } as any); + + const body = $sbaModal.confirm.mock.calls[0][1]; + expect(body).toBe('Shutdown application my-app?'); + }); +}); + +describe('InstanceActionHandler', () => { + const maliciousId = ''; + + it('sanitizes the instance id in the shutdown confirmation dialog', async () => { + const $sbaModal = createModalStub(false); + const notificationCenter = createNotificationCenterStub(); + const handler = new InstanceActionHandler($sbaModal, t, notificationCenter); + + await handler.shutdown({ id: maliciousId } as any); + + const body = $sbaModal.confirm.mock.calls[0][1]; + expect(body).not.toContain(' { + const $sbaModal = createModalStub(false); + const notificationCenter = createNotificationCenterStub(); + const handler = new InstanceActionHandler($sbaModal, t, notificationCenter); + + await handler.restart({ id: maliciousId } as any); + + const body = $sbaModal.confirm.mock.calls[0][1]; + expect(body).not.toContain(' { + const $sbaModal = createModalStub(true); + const notificationCenter = createNotificationCenterStub(); + const instance = { + id: maliciousId, + unregister: vi.fn().mockResolvedValue(undefined), + }; + const handler = new InstanceActionHandler($sbaModal, t, notificationCenter); + + await handler.unregister(instance as any); + + const confirmBody = $sbaModal.confirm.mock.calls[0][1]; + expect(confirmBody).not.toContain(' { + const $sbaModal = createModalStub(true); + const notificationCenter = createNotificationCenterStub(); + const instance = { + id: maliciousId, + unregister: vi.fn().mockRejectedValue({ response: { status: 500 } }), + }; + const handler = new InstanceActionHandler($sbaModal, t, notificationCenter); + + await handler.unregister(instance as any); + + const errorMessage = notificationCenter.error.mock.calls[0][0]; + expect(errorMessage).not.toContain('; @@ -19,7 +20,7 @@ export class InstanceActionHandler implements ActionHandler { async unregister(item: Instance) { const isConfirmed = await this.$sbaModal.confirm( this.t('applications.actions.unregister'), - this.t('instances.unregister', { name: item.id }), + sanitizeHtml(this.t('instances.unregister', { name: item.id })), ); if (!isConfirmed) { return; @@ -28,14 +29,18 @@ export class InstanceActionHandler implements ActionHandler { try { await item.unregister(); this.notificationCenter.success( - this.t('instances.unregister_successful', { name: item.id }), + sanitizeHtml( + this.t('instances.unregister_successful', { name: item.id }), + ), ); } catch (error) { this.notificationCenter.error( - this.t('instances.unregister_failed', { - name: item.id || item.name, - error: error.response.status, - }), + sanitizeHtml( + this.t('instances.unregister_failed', { + name: item.id || item.name, + error: error.response.status, + }), + ), ); } } @@ -43,7 +48,7 @@ export class InstanceActionHandler implements ActionHandler { async shutdown(item: Instance) { const isConfirmed = await this.$sbaModal.confirm( this.t('applications.actions.shutdown'), - this.t('instances.shutdown', { name: item.id }), + sanitizeHtml(this.t('instances.shutdown', { name: item.id })), ); if (!isConfirmed) { return; @@ -52,14 +57,18 @@ export class InstanceActionHandler implements ActionHandler { try { await item.shutdown(); this.notificationCenter.success( - this.t('instances.shutdown_successful', { name: item.id }), + sanitizeHtml( + this.t('instances.shutdown_successful', { name: item.id }), + ), ); } catch (error) { this.notificationCenter.error( - this.t('instances.shutdown_failed', { - name: item.id || item.name, - error: error.response.status, - }), + sanitizeHtml( + this.t('instances.shutdown_failed', { + name: item.id || item.name, + error: error.response.status, + }), + ), ); } } @@ -67,7 +76,7 @@ export class InstanceActionHandler implements ActionHandler { async restart(item: Instance) { const isConfirmed = await this.$sbaModal.confirm( this.t('applications.actions.restart'), - this.t('instances.restart', { name: item.id }), + sanitizeHtml(this.t('instances.restart', { name: item.id })), ); if (!isConfirmed) { return; @@ -76,14 +85,16 @@ export class InstanceActionHandler implements ActionHandler { try { await item.restart(); this.notificationCenter.success( - this.t('instances.restarted', { name: item.id }), + sanitizeHtml(this.t('instances.restarted', { name: item.id })), ); } catch (error) { this.notificationCenter.error( - this.t('instances.restart_failed', { - name: item.id || item.name, - error: error.response.status, - }), + sanitizeHtml( + this.t('instances.restart_failed', { + name: item.id || item.name, + error: error.response.status, + }), + ), ); } } @@ -99,7 +110,7 @@ export class ApplicationActionHandler implements ActionHandler { async restart(application: Application) { const isConfirmed = await this.$sbaModal.confirm( this.t('applications.actions.restart'), - this.t('applications.restart', { name: application.name }), + sanitizeHtml(this.t('applications.restart', { name: application.name })), ); if (!isConfirmed) { return; @@ -108,14 +119,18 @@ export class ApplicationActionHandler implements ActionHandler { try { await application.restart(); this.notificationCenter.success( - this.t('applications.restarted', { name: application.name }), + sanitizeHtml( + this.t('applications.restarted', { name: application.name }), + ), ); } catch (error) { this.notificationCenter.error( - this.t('applications.restart_failed', { - name: application.name, - error: error.response.status, - }), + sanitizeHtml( + this.t('applications.restart_failed', { + name: application.name, + error: error.response.status, + }), + ), ); } } @@ -123,7 +138,7 @@ export class ApplicationActionHandler implements ActionHandler { async shutdown(application: Application) { const isConfirmed = await this.$sbaModal.confirm( this.t('applications.actions.shutdown'), - this.t('applications.shutdown', { name: application.name }), + sanitizeHtml(this.t('applications.shutdown', { name: application.name })), ); if (!isConfirmed) { return; @@ -132,14 +147,20 @@ export class ApplicationActionHandler implements ActionHandler { try { await application.shutdown(); this.notificationCenter.success( - this.t('applications.shutdown_successful', { name: application.name }), + sanitizeHtml( + this.t('applications.shutdown_successful', { + name: application.name, + }), + ), ); } catch (error) { this.notificationCenter.error( - this.t('applications.shutdown_failed', { - name: application.name, - error: error.response.status, - }), + sanitizeHtml( + this.t('applications.shutdown_failed', { + name: application.name, + error: error.response.status, + }), + ), ); } } @@ -147,7 +168,9 @@ export class ApplicationActionHandler implements ActionHandler { async unregister(application: Application) { const isConfirmed = await this.$sbaModal.confirm( this.t('applications.actions.unregister'), - this.t('applications.unregister', { name: application.name }), + sanitizeHtml( + this.t('applications.unregister', { name: application.name }), + ), ); if (!isConfirmed) { return; @@ -156,16 +179,20 @@ export class ApplicationActionHandler implements ActionHandler { try { await application.unregister(); this.notificationCenter.success( - this.t('applications.unregister_successful', { - name: application.name, - }), + sanitizeHtml( + this.t('applications.unregister_successful', { + name: application.name, + }), + ), ); } catch (error) { this.notificationCenter.error( - this.t('applications.unregister_failed', { - name: application.name, - error: error.response.status, - }), + sanitizeHtml( + this.t('applications.unregister_failed', { + name: application.name, + error: error.response.status, + }), + ), ); } } diff --git a/spring-boot-admin-server-ui/src/main/frontend/views/applications/NotificationFilterSettings.spec.ts b/spring-boot-admin-server-ui/src/main/frontend/views/applications/NotificationFilterSettings.spec.ts new file mode 100644 index 00000000000..bdc57a83505 --- /dev/null +++ b/spring-boot-admin-server-ui/src/main/frontend/views/applications/NotificationFilterSettings.spec.ts @@ -0,0 +1,82 @@ +/* + * Copyright 2014-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { screen } from '@testing-library/vue'; +import { describe, expect, it } from 'vitest'; + +import { render } from '@/test-utils'; +import NotificationFilterSettings from '@/views/applications/NotificationFilterSettings.vue'; + +describe('NotificationFilterSettings', () => { + it('strips dangerous markup contained in the object name', async () => { + const maliciousName = 'rogue-app'; + + render(NotificationFilterSettings, { + props: { + object: { name: maliciousName }, + notificationFilters: [], + }, + }); + + // The application name still shows up as text ... + expect(await screen.findByText(/rogue-app/)).toBeInTheDocument(); + + // ... but the must have been stripped by sanitizeHtml, + // it must never be parsed into a real element / fire its handler. + expect(document.querySelectorAll('img').length).toBe(0); + expect((window as any).__xss).toBeUndefined(); + }); + + it('renders safe HTML markup contained in the object id', async () => { + const nameWithMarkup = 'bold-instance'; + + render(NotificationFilterSettings, { + props: { + object: { id: nameWithMarkup }, + notificationFilters: [ + { + affects: () => true, + expiry: null, + }, + ], + }, + }); + + // sanitize-html allows harmless formatting tags like by default, + // so they render as real elements ... + const bold = await screen.findByText('bold-instance'); + expect(bold.tagName).toBe('B'); + }); + + it('strips script tags contained in the object id', async () => { + const maliciousId = 'evil-instance'; + + render(NotificationFilterSettings, { + props: { + object: { id: maliciousId }, + notificationFilters: [ + { + affects: () => true, + expiry: null, + }, + ], + }, + }); + + expect(await screen.findByText(/evil-instance/)).toBeInTheDocument(); + expect(document.querySelectorAll('script').length).toBe(0); + expect((window as any).__xss).toBeUndefined(); + }); +}); diff --git a/spring-boot-admin-server-ui/src/main/frontend/views/applications/NotificationFilterSettings.vue b/spring-boot-admin-server-ui/src/main/frontend/views/applications/NotificationFilterSettings.vue index 0f7af2f719f..1c2c5b56135 100644 --- a/spring-boot-admin-server-ui/src/main/frontend/views/applications/NotificationFilterSettings.vue +++ b/spring-boot-admin-server-ui/src/main/frontend/views/applications/NotificationFilterSettings.vue @@ -21,9 +21,11 @@

    import { useI18n } from 'vue-i18n'; +import { sanitizeHtml } from '@/utils/sanitizeHtml'; + export default { props: { object: { @@ -102,6 +108,7 @@ export default { return { t: i18n.t, currentLocale: i18n.locale, + sanitizeHtml, }; }, data() { diff --git a/spring-boot-admin-server-ui/src/main/frontend/views/instances/details/health-details.spec.ts b/spring-boot-admin-server-ui/src/main/frontend/views/instances/details/health-details.spec.ts index eff6ae53575..02093a781e2 100644 --- a/spring-boot-admin-server-ui/src/main/frontend/views/instances/details/health-details.spec.ts +++ b/spring-boot-admin-server-ui/src/main/frontend/views/instances/details/health-details.spec.ts @@ -513,5 +513,36 @@ describe('HealthDetails', () => { const toggleButton = screen.queryByRole('button'); expect(toggleButton).not.toBeInTheDocument(); }); + + it('should not render HTML/script markup contained in a string detail value', async () => { + const maliciousValue = + ''; + + const healthMock = { + status: 'UP', + details: { + canary: maliciousValue, + }, + }; + + render(HealthDetails, { + props: { + name: 'db', + health: healthMock, + instance: mockInstance, + }, + }); + + const canaryDetail = await screen.findByRole('definition', { + name: 'canary', + }); + + // sanitize-html strips disallowed tags (e.g. ) entirely, so no + // markup and no onerror handler must reach the DOM. + expect(canaryDetail.innerHTML).not.toContain(' @@ -124,6 +124,7 @@ import SbaFormattedObj from '@/components/sba-formatted-obj.vue'; import Instance from '@/services/instance'; import autolink from '@/utils/autolink'; +import { sanitizeHtml } from '@/utils/sanitizeHtml'; const { t } = useI18n(); const id = useId(); diff --git a/spring-boot-admin-server-ui/src/main/frontend/views/instances/env/refresh.vue b/spring-boot-admin-server-ui/src/main/frontend/views/instances/env/refresh.vue index 5e5f82d32a1..0b8920308ec 100644 --- a/spring-boot-admin-server-ui/src/main/frontend/views/instances/env/refresh.vue +++ b/spring-boot-admin-server-ui/src/main/frontend/views/instances/env/refresh.vue @@ -21,8 +21,29 @@