diff --git a/src/Umbraco.Web.UI.Client/devops/eslint/rules/no-unsafe-localize.cjs b/src/Umbraco.Web.UI.Client/devops/eslint/rules/no-unsafe-localize.cjs new file mode 100644 index 000000000000..61d91edad680 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/devops/eslint/rules/no-unsafe-localize.cjs @@ -0,0 +1,70 @@ +'use strict'; + +/** + * Flags `unsafeHTML(.localize.string(...))` and `unsafeHTML(.localize.term(...))`. + * + * Wrapping a localized string in `unsafeHTML` leaves any interpolated args un-escaped — an XSS hazard + * when the args are user-controlled. Use `.localize.htmlString(text, ...args)` instead, which + * escapes args via escapeHTML and returns a Lit unsafeHTML directive ready to inline in templates. + * + * See `docs/security.md` (XSS Prevention → Localized HTML) for the full pattern. + */ + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'Disallow `unsafeHTML(.localize.string|term(...))`. Use `.localize.htmlString(...)` instead.', + category: 'Possible Errors', + recommended: true, + }, + schema: [], + messages: { + unsafeLocalize: + 'Avoid `unsafeHTML(...localize.{{method}}(...))` — interpolated args are not escaped (XSS hazard). Use `localize.htmlString(...)` instead.', + }, + }, + create(context) { + /** + * Returns true if the given AST node represents a `<...>.localize` member access, + * e.g. `this.localize`, `this.#localize`, `host.localize`, `this._localize`. + */ + function isLocalizeMemberAccess(node) { + if (!node || node.type !== 'MemberExpression') return false; + const prop = node.property; + if (!prop) return false; + // Match `localize` (regular) or any private/aliased identifier ending in `localize` (e.g. `#localize`, `_localize`). + if (prop.type === 'Identifier' && /localize$/i.test(prop.name)) return true; + if (prop.type === 'PrivateIdentifier' && /localize$/i.test(prop.name)) return true; + return false; + } + + return { + CallExpression(node) { + // Looking for: unsafeHTML() + if (!node.callee || node.callee.type !== 'Identifier' || node.callee.name !== 'unsafeHTML') return; + if (node.arguments.length === 0) return; + + const arg = node.arguments[0]; + if (!arg || arg.type !== 'CallExpression') return; + + // must itself be a member-call: .string(...) or .term(...) + const innerCallee = arg.callee; + if (!innerCallee || innerCallee.type !== 'MemberExpression') return; + if (innerCallee.property?.type !== 'Identifier') return; + + const method = innerCallee.property.name; + if (method !== 'string' && method !== 'term') return; + + if (!isLocalizeMemberAccess(innerCallee.object)) return; + + context.report({ + node, + messageId: 'unsafeLocalize', + data: { method }, + }); + }, + }; + }, +}; diff --git a/src/Umbraco.Web.UI.Client/docs/security.md b/src/Umbraco.Web.UI.Client/docs/security.md index 28356aca992d..01176344497f 100644 --- a/src/Umbraco.Web.UI.Client/docs/security.md +++ b/src/Umbraco.Web.UI.Client/docs/security.md @@ -1,9 +1,9 @@ # Security + [← Umbraco Backoffice](../CLAUDE.md) | [← Monorepo Root](../../CLAUDE.md) --- - ### Input Validation **Validate All User Input**: @@ -40,27 +40,10 @@ private _validateName(name: string): boolean { } ``` -**Sanitize HTML**: - -```typescript -// Use the sanitizeHTML utility, which wraps DOMPurify internally -import { sanitizeHTML } from '@umbraco-cms/backoffice/utils'; - -const cleanHtml = sanitizeHTML(userInput); - -// In Lit templates, use unsafeHTML directive with sanitized content -import { unsafeHTML } from '@umbraco-cms/backoffice/external/lit'; - -render() { - return html`
${unsafeHTML(sanitizeHTML(this.htmlContent))}
`; -} -``` - -> **Note**: Do not import or call `DOMPurify` directly. Always use `sanitizeHTML` from `@umbraco-cms/backoffice/utils`. - ### Authentication & Authorization **OpenID Connect with PKCE** (v17+): + - Backoffice uses PKCE authorization code flow - Real tokens are stored exclusively in `__Host-umbAccessToken` / `__Host-umbRefreshToken` httpOnly cookies — JavaScript cannot read them - The client-side bearer token value is always the literal string `'[redacted]'` — the server (`HideBackOfficeTokensHandler`) swaps it for the real cookie on each request @@ -99,6 +82,7 @@ async #handleDelete() { ``` **Context Security**: + - Use Context API for auth state (`UMB_AUTH_CONTEXT`) - Never store tokens in localStorage, sessionStorage, or JS variables - Backend handles token refresh via httpOnly cookies @@ -123,11 +107,13 @@ const response = await client.getById({ id }); ``` **CORS** (Backend Configuration): + - Configured in .NET backend - Backoffice follows same-origin policy - API calls to same origin **Rate Limiting** (Backend): + - Handled by .NET backend - Backoffice respects rate limit headers @@ -136,16 +122,58 @@ const response = await client.getById({ id }); **Template Security** (Lit): ```typescript +import { html, unsafeHTML } from '@umbraco-cms/backoffice/external/lit'; +import { sanitizeHTML, escapeHTML } from '@umbraco-cms/backoffice/utils'; + // Lit automatically escapes content in templates render() { + + // UNSAFE - This is only safe if you are 100% sure that htmlContent is sanitized properly before being set, and that it cannot be manipulated by user input in any way. Use with extreme caution. + return html`
${unsafeHTML(this.htmlContent)}
`; + // Safe - Automatically escaped return html`
${this.userContent}
`; - // UNSAFE - Only use with sanitized content - return html`
${unsafeHTML(DOMPurify.sanitize(this.htmlContent))}
`; + // Safe - Use with sanitized content + return html`
${unsafeHTML(sanitizeHTML(this.htmlContent))}
`; + + // Safe - Use with escaped content, which is essentially what Lit does by default + return html`
${unsafeHTML(escapeHTML(this.htmlContent))}
`; + + // Safe - localize.htmlString() escapes interpolated args and wraps in unsafeHTML for rendering + return html`
${this.localize.htmlString('#someKey_withHtml', this.userContent)}
`; + + // Safe - component automatically escapes arguments + return html``; } ``` +**Localized HTML — `localize.string()` vs `localize.htmlString()`**: + +- `localize.string(text, ...args)` — returns a plain string. Use for **non-HTML** contexts: attribute bindings (Lit auto-escapes), notification messages, button labels, log strings. Args are NOT escaped because Lit (or the consumer) handles the appropriate escaping for the context. +- `localize.htmlString(text, ...args)` — returns a Lit directive that renders via `unsafeHTML` with all args HTML-escaped. Use whenever the localized value contains HTML markup that must be rendered (e.g. `` links, `` emphasis) — this is the only safe path when interpolating user-controlled args into HTML output. + +```typescript +// ✅ Plain text — string() is correct (Lit escapes the attribute itself) +html``; + +// ✅ HTML rendering — htmlString() escapes args + wraps in unsafeHTML +html`

${this.localize.htmlString('#defaultdialogs_confirmdelete', userControlledName)}

`; + +// ❌ Manually combining string() + unsafeHTML leaves args un-escaped — XSS hazard +html`

${unsafeHTML(this.localize.string('#defaultdialogs_confirmdelete', userControlledName))}

`; +``` + +**Modal `content` field** (e.g. `umbConfirmModal`) renders strings via `unsafeHTML` internally. When passing a localized string with user-controlled args, wrap it in a template: + +```typescript +// ✅ Safe — htmlString escapes args, html`...` wraps the directive in a TemplateResult +umbConfirmModal(this, { + headline: '#actions_delete', + content: html`${this.#localize.htmlString('#defaultdialogs_confirmdelete', item.name)}`, +}); +``` + **Attribute Binding**: ```typescript @@ -172,12 +200,14 @@ render() { ### Content Security Policy **CSP Headers** (Backend Configuration): + - Configured in .NET backend - Restricts script sources - Prevents inline scripts (except with nonce) - Reports violations **Backoffice Compliance**: + - No inline scripts - No `eval()` or `Function()` constructor - Monaco Editor uses web workers (CSP compliant) @@ -198,6 +228,7 @@ npm update ``` **Dependency Security Practices**: + - Renovate bot automatically creates PRs for updates - Review dependency changes before merging - Only use packages from npm registry @@ -205,33 +236,39 @@ npm update - Keep dependencies updated **Known Vulnerabilities**: + - CI checks for vulnerabilities on every PR - Security advisories reviewed regularly ### Common Vulnerabilities **XSS (Cross-Site Scripting)**: + - ✅ Lit templates automatically escape content - ✅ DOMPurify for HTML sanitization - ❌ Never use `unsafeHTML` with user input directly - ❌ Never set `innerHTML` with user input **CSRF (Cross-Site Request Forgery)**: + - ✅ Backend sends CSRF tokens - ✅ OpenAPI client includes tokens automatically - ✅ SameSite cookies **Injection Attacks**: + - ✅ Backend uses parameterized queries - ✅ Input validation on both frontend and backend - ✅ OpenAPI client prevents injection **Prototype Pollution**: + - ❌ Never use `Object.assign` with user input as source - ❌ Never use `_.merge` with untrusted data - ✅ Validate object shapes before using **ReDoS (Regular Expression Denial of Service)**: + - ✅ Review complex regex patterns - ✅ Test regex with long inputs - ❌ Avoid backtracking in regex @@ -239,25 +276,30 @@ npm update ### Secure Coding Practices **Don't Trust Client Data**: + - Validate on backend (primary defense) - Frontend validation is UX, not security **Principle of Least Privilege**: + - Only request permissions needed - Check permissions before sensitive operations - Hide UI for unavailable actions **Sanitize Output**: + - Always sanitize HTML before rendering - Escape special characters in user content - Use Lit's automatic escaping **Secure Defaults**: + - Forms should validate by default - Sensitive operations require confirmation - Errors don't expose sensitive information **Defense in Depth**: + - Multiple layers of security - Frontend validation + Backend validation - Input sanitization + Output escaping @@ -266,6 +308,7 @@ npm update ### Security Anti-Patterns to Avoid ❌ **Never do this**: + ```typescript // XSS vulnerability element.innerHTML = userInput; @@ -289,6 +332,7 @@ window.location.href = url; ``` ✅ **Do this instead**: + ```typescript // Safe HTML rendering element.textContent = userInput; @@ -315,5 +359,3 @@ if (url.protocol === 'https:' || url.protocol === 'http:') { window.location.href = url.href; } ``` - - diff --git a/src/Umbraco.Web.UI.Client/eslint-local-rules.cjs b/src/Umbraco.Web.UI.Client/eslint-local-rules.cjs index 446d44978e31..d4f515ee84b7 100644 --- a/src/Umbraco.Web.UI.Client/eslint-local-rules.cjs +++ b/src/Umbraco.Web.UI.Client/eslint-local-rules.cjs @@ -7,6 +7,7 @@ const noDirectApiImportRule = require('./devops/eslint/rules/no-direct-api-impor const preferImportAliasesRule = require('./devops/eslint/rules/prefer-import-aliases.cjs'); const preferStaticStylesLastRule = require('./devops/eslint/rules/prefer-static-styles-last.cjs'); const noRelativeImportToImportMapModule = require('./devops/eslint/rules/no-relative-import-to-import-map-module.cjs'); +const noUnsafeLocalize = require('./devops/eslint/rules/no-unsafe-localize.cjs'); const enforceManifestAliasRule = require('./devops/eslint/rules/enforce-manifest-alias.cjs'); module.exports = { @@ -17,5 +18,6 @@ module.exports = { 'prefer-import-aliases': preferImportAliasesRule, 'prefer-static-styles-last': preferStaticStylesLastRule, 'no-relative-import-to-import-map-module': noRelativeImportToImportMapModule, + 'no-unsafe-localize': noUnsafeLocalize, 'enforce-manifest-alias': enforceManifestAliasRule, }; diff --git a/src/Umbraco.Web.UI.Client/eslint.config.js b/src/Umbraco.Web.UI.Client/eslint.config.js index 696cf16ef029..a423c5099d89 100644 --- a/src/Umbraco.Web.UI.Client/eslint.config.js +++ b/src/Umbraco.Web.UI.Client/eslint.config.js @@ -55,6 +55,7 @@ export default [ 'import/no-cycle': ['error', { maxDepth: 6, allowUnsafeDynamicCyclicDependency: true }], 'local-rules/enforce-manifest-alias': 'warn', 'local-rules/prefer-static-styles-last': 'warn', + 'local-rules/no-unsafe-localize': 'error', 'local-rules/enforce-umbraco-external-imports': [ 'error', { diff --git a/src/Umbraco.Web.UI.Client/src/apps/installer/consent/installer-consent.element.ts b/src/Umbraco.Web.UI.Client/src/apps/installer/consent/installer-consent.element.ts index f8ac5ad7fd2f..3809908e66f9 100644 --- a/src/Umbraco.Web.UI.Client/src/apps/installer/consent/installer-consent.element.ts +++ b/src/Umbraco.Web.UI.Client/src/apps/installer/consent/installer-consent.element.ts @@ -2,6 +2,7 @@ import type { UmbInstallerContext } from '../installer.context.js'; import { UMB_INSTALLER_CONTEXT } from '../installer.context.js'; import type { CSSResultGroup } from '@umbraco-cms/backoffice/external/lit'; import { css, html, customElement, state, unsafeHTML } from '@umbraco-cms/backoffice/external/lit'; +import { sanitizeHTML } from '@umbraco-cms/backoffice/utils'; import type { ConsentLevelPresentationModel, @@ -73,7 +74,9 @@ export class UmbInstallerConsentElement extends UmbLitElement { private _renderSlider() { if (!this._telemetryLevels || this._telemetryLevels.length < 1) return; - + const sanitizedDescription = this._selectedTelemetry?.description + ? sanitizeHTML(this._selectedTelemetry.description) + : ''; return html`

${this._selectedTelemetry.level}

-

${unsafeHTML(this._selectedTelemetry.description)}

+

${unsafeHTML(sanitizedDescription)}

`; } diff --git a/src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.test.ts b/src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.test.ts index 55fed2d0a670..e82e81bf947d 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.test.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.test.ts @@ -453,6 +453,23 @@ describe('UmbLocalizationController', () => { }); }); + describe('htmlString', () => { + it('should HTML-escape interpolated arguments', async () => { + const xss = ''; + // Render the directive into an element to inspect the resulting innerHTML + const host = await fixture(html`
${controller.htmlString('#withInlineToken', xss, '')}
`); + expect(host.innerHTML, 'XSS detected').to.not.contain('' }; + const host = await fixture(html`
${controller.htmlString('#withInlineToken', xss, '')}
`); + expect(host.innerHTML, 'XSS via toString detected').to.not.contain('