Skip to content

Commit

Permalink
UI: Remove usage of htmlSafe (hashicorp#20235)
Browse files Browse the repository at this point in the history
  • Loading branch information
hashishaw authored Apr 19, 2023
1 parent 98786d9 commit fae1bff
Show file tree
Hide file tree
Showing 17 changed files with 76 additions and 24 deletions.
3 changes: 3 additions & 0 deletions changelog/20235.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: remove use of htmlSafe except when first sanitized
```
5 changes: 2 additions & 3 deletions ui/app/components/diff-version-selector.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import Component from '@glimmer/component';
import { inject as service } from '@ember/service';
import { action } from '@ember/object';
import { tracked } from '@glimmer/tracking';
import { htmlSafe } from '@ember/template';

/**
* @module DiffVersionSelector
Expand Down Expand Up @@ -64,10 +63,10 @@ export default class DiffVersionSelector extends Component {
if (delta === undefined) {
this.statesMatch = true;
// 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 {
this.statesMatch = false;
this.visualDiff = htmlSafe(jsondiffpatch.formatters.html.format(delta, rightSideVersionData));
this.visualDiff = jsondiffpatch.formatters.html.format(delta, rightSideVersionData);
}
}

Expand Down
9 changes: 4 additions & 5 deletions ui/app/components/wizard-content.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { inject as service } from '@ember/service';
import Component from '@ember/component';
import { computed } from '@ember/object';
import { FEATURE_MACHINE_STEPS, INIT_STEPS } from 'vault/helpers/wizard-constants';
import { htmlSafe } from '@ember/template';

export default Component.extend({
wizard: service(),
Expand Down Expand Up @@ -89,25 +88,25 @@ export default Component.extend({
const bar = [];
if (this.currentTutorialProgress) {
bar.push({
style: htmlSafe(`width:${this.currentTutorialProgress.percentage}%;`),
style: `width:${this.currentTutorialProgress.percentage}%;`,
completed: false,
showIcon: true,
});
} else {
if (this.currentFeatureProgress) {
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) => {
if (feature === this.currentMachine) {
bar.push({
style: htmlSafe(`width:${this.currentFeatureProgress.percentage}%;`),
style: `width:${this.currentFeatureProgress.percentage}%;`,
completed: this.currentFeatureProgress.percentage == 100 ? true : false,
feature: feature,
showIcon: true,
});
} else {
bar.push({ style: htmlSafe('width:0%;'), completed: false, feature: feature, showIcon: true });
bar.push({ style: 'width:0%;', completed: false, feature: feature, showIcon: true });
}
});
}
Expand Down
5 changes: 2 additions & 3 deletions ui/app/components/wizard/features-selection.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { inject as service } from '@ember/service';
import Component from '@ember/component';
import { computed } from '@ember/object';
import { FEATURE_MACHINE_TIME } from 'vault/helpers/wizard-constants';
import { htmlSafe } from '@ember/template';

export default Component.extend({
wizard: service(),
Expand Down Expand Up @@ -55,10 +54,10 @@ export default Component.extend({
}),
selectProgress: computed('selectedFeatures', function () {
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) {
bar = [{ style: htmlSafe('width:0%;'), showIcon: false }];
bar = [{ style: 'width:0%;', showIcon: false }];
}
return bar;
}),
Expand Down
2 changes: 1 addition & 1 deletion ui/app/templates/components/diff-version-selector.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -98,5 +98,5 @@
</Toolbar>

<div class="form-section visual-diff">
<pre>{{this.visualDiff}}</pre>
<pre>{{sanitized-html this.visualDiff}}</pre>
</div>
2 changes: 1 addition & 1 deletion ui/app/templates/components/wizard-progress.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{{#each @progressBar as |bar|}}
<div class="feature-progress-container">
<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>
{{#if bar.showIcon}}
<Icon @name="check-circle-fill" class="feature-check {{if bar.completed 'completed-check' 'incomplete-check'}}" />
Expand Down
3 changes: 1 addition & 2 deletions ui/lib/core/addon/components/confirm.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import Component from '@ember/component';
import { computed } from '@ember/object';
import { htmlSafe } from '@ember/template';
import layout from '../templates/components/confirm';
import { next } from '@ember/runloop';

Expand Down Expand Up @@ -34,7 +33,7 @@ export default Component.extend({
height: 0,
focusTrigger: null,
style: computed('height', function () {
return htmlSafe(`height: ${this.height}px`);
return `height: ${this.height}px`;
}),
wormholeReference: null,
wormholeId: computed('elementId', function () {
Expand Down
7 changes: 2 additions & 5 deletions ui/lib/core/addon/components/replication-dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import Component from '@ember/component';
import { computed } from '@ember/object';
import { clusterStates } from 'core/helpers/cluster-states';
import { capitalize } from '@ember/string';
import { htmlSafe } from '@ember/template';
import layout from '../templates/components/replication-dashboard';

/**
Expand Down Expand Up @@ -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 {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 {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.
*/

Expand Down Expand Up @@ -94,9 +93,7 @@ export default Component.extend({
}),
reindexMessage: computed('isSecondary', 'progressBar', function () {
if (!this.isSecondary) {
return htmlSafe(
'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 <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.';
}),
Expand Down
14 changes: 14 additions & 0 deletions ui/lib/core/addon/helpers/sanitized-html.js
Original file line number Diff line number Diff line change
@@ -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;
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
@title={{concat "Re-indexing in progress" this.reindexingStage}}
@type="info"
@progressBar={{this.progressBar}}
@message={{this.reindexMessage}}
@message={{sanitized-html this.reindexMessage}}
data-test-isReindexing
/>
</div>
Expand Down
1 change: 1 addition & 0 deletions ui/lib/core/app/helpers/sanitized-html.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from 'core/helpers/sanitized-html';
1 change: 1 addition & 0 deletions ui/lib/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"date-fns": "*",
"@icholy/duration": "*",
"base64-js": "*",
"dompurify": "*",
"ember-auto-import": "*",
"ember-basic-dropdown": "*",
"ember-cli-babel": "*",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@
@value={{template.rules}}
@mode="ruby"
@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>
Restore example
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { action } from '@ember/object';
import { task } from 'ember-concurrency';
import { waitFor } from '@ember/test-waiters';
import { getRules } from '../../../utils/generated-role-rules';
import { htmlSafe } from '@ember/template';
import errorMessage from 'vault/utils/error-message';

/**
Expand Down Expand Up @@ -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';
const link =
'<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
Expand Down
1 change: 1 addition & 0 deletions ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@
"date-fns-tz": "^1.2.2",
"deepmerge": "^4.0.0",
"doctoc": "^2.2.0",
"dompurify": "^3.0.2",
"ember-auto-import": "2.4.2",
"ember-basic-dropdown": "6.0.1",
"ember-cli": "~4.4.0",
Expand Down
35 changes: 35 additions & 0 deletions ui/tests/integration/helpers/sanitized-html-test.js
Original file line number Diff line number Diff line change
@@ -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'); }");
});
});
5 changes: 5 additions & 0 deletions ui/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8813,6 +8813,11 @@ domhandler@^5.0.1, domhandler@^5.0.2, domhandler@^5.0.3:
dependencies:
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:
version "1.7.0"
resolved "https://registry.yarnpkg.com/domutils/-/domutils-1.7.0.tgz#56ea341e834e06e6748af7a1cb25da67ea9f8c2a"
Expand Down

0 comments on commit fae1bff

Please sign in to comment.