UI: Remove usage of htmlSafe (#20235)

This commit is contained in:
Chelsea Shaw
2023-04-19 10:40:33 -05:00
committed by GitHub
parent 98786d96c7
commit fae1bffd1c
17 changed files with 76 additions and 24 deletions

3
changelog/20235.txt Normal file
View File

@@ -0,0 +1,3 @@
```release-note:bug
ui: remove use of htmlSafe except when first sanitized
```

View File

@@ -8,7 +8,6 @@ import Component from '@glimmer/component';
import { inject as service } from '@ember/service'; import { inject as service } from '@ember/service';
import { action } from '@ember/object'; import { action } from '@ember/object';
import { tracked } from '@glimmer/tracking'; import { tracked } from '@glimmer/tracking';
import { htmlSafe } from '@ember/template';
/** /**
* @module DiffVersionSelector * @module DiffVersionSelector
@@ -64,10 +63,10 @@ export default class DiffVersionSelector extends Component {
if (delta === undefined) { if (delta === undefined) {
this.statesMatch = true; this.statesMatch = true;
// params: value, replacer (all properties included), space (white space and indentation, line break, etc.) // params: value, replacer (all properties included), space (white space and indentation, line break, etc.)
this.visualDiff = htmlSafe(JSON.stringify(leftSideVersionData, undefined, 2)); this.visualDiff = JSON.stringify(leftSideVersionData, undefined, 2);
} else { } else {
this.statesMatch = false; this.statesMatch = false;
this.visualDiff = htmlSafe(jsondiffpatch.formatters.html.format(delta, rightSideVersionData)); this.visualDiff = jsondiffpatch.formatters.html.format(delta, rightSideVersionData);
} }
} }

View File

@@ -8,7 +8,6 @@ import { inject as service } from '@ember/service';
import Component from '@ember/component'; import Component from '@ember/component';
import { computed } from '@ember/object'; import { computed } from '@ember/object';
import { FEATURE_MACHINE_STEPS, INIT_STEPS } from 'vault/helpers/wizard-constants'; import { FEATURE_MACHINE_STEPS, INIT_STEPS } from 'vault/helpers/wizard-constants';
import { htmlSafe } from '@ember/template';
export default Component.extend({ export default Component.extend({
wizard: service(), wizard: service(),
@@ -89,25 +88,25 @@ export default Component.extend({
const bar = []; const bar = [];
if (this.currentTutorialProgress) { if (this.currentTutorialProgress) {
bar.push({ bar.push({
style: htmlSafe(`width:${this.currentTutorialProgress.percentage}%;`), style: `width:${this.currentTutorialProgress.percentage}%;`,
completed: false, completed: false,
showIcon: true, showIcon: true,
}); });
} else { } else {
if (this.currentFeatureProgress) { if (this.currentFeatureProgress) {
this.completedFeatures.forEach((feature) => { this.completedFeatures.forEach((feature) => {
bar.push({ style: htmlSafe('width:100%;'), completed: true, feature: feature, showIcon: true }); bar.push({ style: 'width:100%;', completed: true, feature: feature, showIcon: true });
}); });
this.wizard.featureList.forEach((feature) => { this.wizard.featureList.forEach((feature) => {
if (feature === this.currentMachine) { if (feature === this.currentMachine) {
bar.push({ bar.push({
style: htmlSafe(`width:${this.currentFeatureProgress.percentage}%;`), style: `width:${this.currentFeatureProgress.percentage}%;`,
completed: this.currentFeatureProgress.percentage == 100 ? true : false, completed: this.currentFeatureProgress.percentage == 100 ? true : false,
feature: feature, feature: feature,
showIcon: true, showIcon: true,
}); });
} else { } else {
bar.push({ style: htmlSafe('width:0%;'), completed: false, feature: feature, showIcon: true }); bar.push({ style: 'width:0%;', completed: false, feature: feature, showIcon: true });
} }
}); });
} }

View File

@@ -8,7 +8,6 @@ import { inject as service } from '@ember/service';
import Component from '@ember/component'; import Component from '@ember/component';
import { computed } from '@ember/object'; import { computed } from '@ember/object';
import { FEATURE_MACHINE_TIME } from 'vault/helpers/wizard-constants'; import { FEATURE_MACHINE_TIME } from 'vault/helpers/wizard-constants';
import { htmlSafe } from '@ember/template';
export default Component.extend({ export default Component.extend({
wizard: service(), wizard: service(),
@@ -55,10 +54,10 @@ export default Component.extend({
}), }),
selectProgress: computed('selectedFeatures', function () { selectProgress: computed('selectedFeatures', function () {
let bar = this.selectedFeatures.map((feature) => { let bar = this.selectedFeatures.map((feature) => {
return { style: htmlSafe('width:0%;'), completed: false, showIcon: true, feature: feature }; return { style: 'width:0%;', completed: false, showIcon: true, feature: feature };
}); });
if (bar.length === 0) { if (bar.length === 0) {
bar = [{ style: htmlSafe('width:0%;'), showIcon: false }]; bar = [{ style: 'width:0%;', showIcon: false }];
} }
return bar; return bar;
}), }),

View File

@@ -98,5 +98,5 @@
</Toolbar> </Toolbar>
<div class="form-section visual-diff"> <div class="form-section visual-diff">
<pre>{{this.visualDiff}}</pre> <pre>{{sanitized-html this.visualDiff}}</pre>
</div> </div>

View File

@@ -2,7 +2,7 @@
{{#each @progressBar as |bar|}} {{#each @progressBar as |bar|}}
<div class="feature-progress-container"> <div class="feature-progress-container">
<span class="progress-bar"> <span class="progress-bar">
<span class="feature-progress" style={{bar.style}} {{! template-lint-disable }}></span> <span class="feature-progress" style={{sanitized-html bar.style}}></span>
</span> </span>
{{#if bar.showIcon}} {{#if bar.showIcon}}
<Icon @name="check-circle-fill" class="feature-check {{if bar.completed 'completed-check' 'incomplete-check'}}" /> <Icon @name="check-circle-fill" class="feature-check {{if bar.completed 'completed-check' 'incomplete-check'}}" />

View File

@@ -5,7 +5,6 @@
import Component from '@ember/component'; import Component from '@ember/component';
import { computed } from '@ember/object'; import { computed } from '@ember/object';
import { htmlSafe } from '@ember/template';
import layout from '../templates/components/confirm'; import layout from '../templates/components/confirm';
import { next } from '@ember/runloop'; import { next } from '@ember/runloop';
@@ -34,7 +33,7 @@ export default Component.extend({
height: 0, height: 0,
focusTrigger: null, focusTrigger: null,
style: computed('height', function () { style: computed('height', function () {
return htmlSafe(`height: ${this.height}px`); return `height: ${this.height}px`;
}), }),
wormholeReference: null, wormholeReference: null,
wormholeId: computed('elementId', function () { wormholeId: computed('elementId', function () {

View File

@@ -7,7 +7,6 @@ import Component from '@ember/component';
import { computed } from '@ember/object'; import { computed } from '@ember/object';
import { clusterStates } from 'core/helpers/cluster-states'; import { clusterStates } from 'core/helpers/cluster-states';
import { capitalize } from '@ember/string'; import { capitalize } from '@ember/string';
import { htmlSafe } from '@ember/template';
import layout from '../templates/components/replication-dashboard'; import layout from '../templates/components/replication-dashboard';
/** /**
@@ -35,7 +34,7 @@ import layout from '../templates/components/replication-dashboard';
* @param {Boolean} [isSummaryDashboard=false] - Only true when the cluster is both a dr and performance primary. If true, replicationDetailsSummary is populated and used to pass through the cluster details. * @param {Boolean} [isSummaryDashboard=false] - Only true when the cluster is both a dr and performance primary. If true, replicationDetailsSummary is populated and used to pass through the cluster details.
* @param {Object} replicationDetailsSummary=null - An Ember data object computed off the Ember Model. It combines the Model.dr and Model.performance objects into one and contains details specific to the mode replication. * @param {Object} replicationDetailsSummary=null - An Ember data object computed off the Ember Model. It combines the Model.dr and Model.performance objects into one and contains details specific to the mode replication.
* @param {Object} replicationDetails=null - An Ember data object pulled from the Ember Model. It contains details specific to the whether the replication is dr or performance. * @param {Object} replicationDetails=null - An Ember data object pulled from the Ember Model. It contains details specific to the whether the replication is dr or performance.
* @param {String} clusterMode=null - The cluster mode passed through to a table component. * @param {String} clusterMode=null - The cluster mode passed through to a table component.
* @param {Object} reindexingDetails=null - An Ember data object used to show a reindexing progress bar. * @param {Object} reindexingDetails=null - An Ember data object used to show a reindexing progress bar.
*/ */
@@ -94,9 +93,7 @@ export default Component.extend({
}), }),
reindexMessage: computed('isSecondary', 'progressBar', function () { reindexMessage: computed('isSecondary', 'progressBar', function () {
if (!this.isSecondary) { if (!this.isSecondary) {
return htmlSafe( return 'This can cause a delay depending on the size of the data store. You can <b>not</b> use Vault during this time.';
'This can cause a delay depending on the size of the data store. You can <b>not</b> use Vault during this time.'
);
} }
return 'This can cause a delay depending on the size of the data store. You can use Vault during this time.'; return 'This can cause a delay depending on the size of the data store. You can use Vault during this time.';
}), }),

View File

@@ -0,0 +1,14 @@
import { helper } from '@ember/component/helper';
import { debug } from '@ember/debug';
import { htmlSafe } from '@ember/template';
import { sanitize } from 'dompurify';
export default helper(function sanitizedHtml([htmlString]) {
try {
return htmlSafe(sanitize(htmlString));
} catch (e) {
debug('Error sanitizing string', e);
// I couldn't get this to actually fail but as a fallback render the value as-is
return htmlString;
}
});

View File

@@ -5,7 +5,7 @@
@title={{concat "Re-indexing in progress" this.reindexingStage}} @title={{concat "Re-indexing in progress" this.reindexingStage}}
@type="info" @type="info"
@progressBar={{this.progressBar}} @progressBar={{this.progressBar}}
@message={{this.reindexMessage}} @message={{sanitized-html this.reindexMessage}}
data-test-isReindexing data-test-isReindexing
/> />
</div> </div>

View File

@@ -0,0 +1 @@
export { default } from 'core/helpers/sanitized-html';

View File

@@ -8,6 +8,7 @@
"date-fns": "*", "date-fns": "*",
"@icholy/duration": "*", "@icholy/duration": "*",
"base64-js": "*", "base64-js": "*",
"dompurify": "*",
"ember-auto-import": "*", "ember-auto-import": "*",
"ember-basic-dropdown": "*", "ember-basic-dropdown": "*",
"ember-cli-babel": "*", "ember-cli-babel": "*",

View File

@@ -109,7 +109,7 @@
@value={{template.rules}} @value={{template.rules}}
@mode="ruby" @mode="ruby"
@valueUpdated={{fn (mut template.rules)}} @valueUpdated={{fn (mut template.rules)}}
@helpText={{this.roleRulesHelpText}} @helpText={{sanitized-html this.roleRulesHelpText}}
> >
<button type="button" class="toolbar-link" {{on "click" this.resetRoleRules}} data-test-restore-example> <button type="button" class="toolbar-link" {{on "click" this.resetRoleRules}} data-test-restore-example>
Restore example Restore example

View File

@@ -10,7 +10,6 @@ import { action } from '@ember/object';
import { task } from 'ember-concurrency'; import { task } from 'ember-concurrency';
import { waitFor } from '@ember/test-waiters'; import { waitFor } from '@ember/test-waiters';
import { getRules } from '../../../utils/generated-role-rules'; import { getRules } from '../../../utils/generated-role-rules';
import { htmlSafe } from '@ember/template';
import errorMessage from 'vault/utils/error-message'; import errorMessage from 'vault/utils/error-message';
/** /**
@@ -87,7 +86,7 @@ export default class CreateAndEditRolePageComponent extends Component {
'This specifies the Role or ClusterRole rules to use when generating a role. Kubernetes documentation is'; 'This specifies the Role or ClusterRole rules to use when generating a role. Kubernetes documentation is';
const link = const link =
'<a href="https://kubernetes.io/docs/reference/access-authn-authz/rbac/" target="_blank" rel="noopener noreferrer">available here</>'; '<a href="https://kubernetes.io/docs/reference/access-authn-authz/rbac/" target="_blank" rel="noopener noreferrer">available here</>';
return htmlSafe(`${message} ${link}.`); return `${message} ${link}.`;
} }
@action @action

View File

@@ -117,6 +117,7 @@
"date-fns-tz": "^1.2.2", "date-fns-tz": "^1.2.2",
"deepmerge": "^4.0.0", "deepmerge": "^4.0.0",
"doctoc": "^2.2.0", "doctoc": "^2.2.0",
"dompurify": "^3.0.2",
"ember-auto-import": "2.4.2", "ember-auto-import": "2.4.2",
"ember-basic-dropdown": "6.0.1", "ember-basic-dropdown": "6.0.1",
"ember-cli": "~4.4.0", "ember-cli": "~4.4.0",

View File

@@ -0,0 +1,35 @@
import { module, test } from 'qunit';
import { setupRenderingTest } from 'vault/tests/helpers';
import { render } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';
module('Integration | Helper | sanitized-html', function (hooks) {
setupRenderingTest(hooks);
test('it does not alter if string is safe', async function (assert) {
this.set('inputValue', 'height: 15.33px');
await render(hbs`{{sanitized-html this.inputValue}}`);
assert.dom(this.element).hasText('height: 15.33px');
});
test('it strips unsafe HTML before rendering safe HTML', async function (assert) {
this.set(
'inputValue',
'<main data-test-thing>This is something<script data-test-script>window.alert(`h4cK3d`)</script></main>'
);
await render(hbs`{{sanitized-html this.inputValue}}`);
assert.dom('[data-test-thing]').hasTagName('main');
assert.dom('[data-test-thing]').hasText('This is something', 'preserves non-problematic content');
assert.dom('[data-test-script]').doesNotExist('Script is stripped from render');
});
test('it does not invoke functions passed as value', async function (assert) {
this.set('inputValue', () => {
window.alert('h4cK3d');
});
await render(hbs`{{sanitized-html this.inputValue}}`);
assert.dom(this.element).hasText("() => { window.alert('h4cK3d'); }");
});
});

View File

@@ -8813,6 +8813,11 @@ domhandler@^5.0.1, domhandler@^5.0.2, domhandler@^5.0.3:
dependencies: dependencies:
domelementtype "^2.3.0" domelementtype "^2.3.0"
dompurify@^3.0.2:
version "3.0.2"
resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-3.0.2.tgz#bc4c7c011c825e7704341a285461d8d407d9429a"
integrity sha512-B8c6JdiEpxAKnd8Dm++QQxJL4lfuc757scZtcapj6qjTjrQzyq5iAyznLKVvK+77eYNiFblHBlt7MM0fOeqoKw==
domutils@^1.7.0: domutils@^1.7.0:
version "1.7.0" version "1.7.0"
resolved "https://registry.yarnpkg.com/domutils/-/domutils-1.7.0.tgz#56ea341e834e06e6748af7a1cb25da67ea9f8c2a" resolved "https://registry.yarnpkg.com/domutils/-/domutils-1.7.0.tgz#56ea341e834e06e6748af7a1cb25da67ea9f8c2a"