Merge pull request #3473 from storybooks/fix-xss

Knobs: add escapeHTML option; use it by default in Vue, Angular, and Polymer

addons/knobs/README.md

@@ -161,6 +161,7 @@ const groupId = 'GROUP-ID1';
 
 const value = text(label, defaultValue, groupId);
 ```
+
 ### boolean
 
 Allows you to get a boolean value from the user.
@@ -385,6 +386,9 @@ const stories = storiesOf('Storybook Knobs', module);
 stories.addDecorator(withKnobsOptions({
   debounce: { wait: number, leading: boolean}, // Same as lodash debounce.
   timestamps: true // Doesn't emit events while user is typing.
+  escapeHTML: true // Escapes strings to be safe for inserting as innerHTML. This option is true by default in storybook for Vue, Angular, and Polymer, because those frameworks allow rendering plain HTML.
+                   // You can still set it to false, but it's strongly unrecommendend in cases when you host your storybook on some route of your main site or web app.
+
 }));
 ```
 

addons/knobs/package.json

@@ -17,6 +17,7 @@
     "@storybook/components": "4.0.0-alpha.3",
     "babel-runtime": "^6.26.0",
     "deep-equal": "^1.0.1",
+    "escape-html": "^1.0.3",
     "global": "^4.3.2",
     "insert-css": "^2.0.0",
     "lodash.debounce": "^4.0.8",

addons/knobs/src/KnobManager.js

@@ -1,19 +1,48 @@
 /* eslint no-underscore-dangle: 0 */
 import deepEqual from 'deep-equal';
+import escape from 'escape-html';
+
 import KnobStore from './KnobStore';
 
 // This is used by _mayCallChannel to determine how long to wait to before triggering a panel update
 const PANEL_UPDATE_INTERVAL = 400;
 
+const escapeStrings = obj => {
+  if (typeof obj === 'string') {
+    return escape(obj);
+  }
+  if (obj == null || typeof obj !== 'object') {
+    return obj;
+  }
+  if (Array.isArray(obj)) {
+    const newArray = obj.map(escapeStrings);
+    const didChange = newArray.some((newValue, key) => newValue !== obj[key]);
+    return didChange ? newArray : obj;
+  }
+  return Object.entries(obj).reduce((acc, [key, oldValue]) => {
+    const newValue = escapeStrings(oldValue);
+    return newValue === oldValue ? acc : { ...acc, [key]: newValue };
+  }, obj);
+};
+
 export default class KnobManager {
   constructor() {
     this.knobStore = new KnobStore();
+    this.options = {};
   }
 
   setChannel(channel) {
     this.channel = channel;
   }
 
+  setOptions(options) {
+    this.options = options;
+  }
+
+  getKnobValue({ value }) {
+    return this.options.escapeHTML ? escapeStrings(value) : value;
+  }
+
   knob(name, options) {
     this._mayCallChannel();
 
@@ -23,7 +52,7 @@ export default class KnobManager {
     // But, if the user changes the code for the defaultValue we should set
     // that value instead.
     if (existingKnob && deepEqual(options.value, existingKnob.defaultValue)) {
-      return existingKnob.value;
+      return this.getKnobValue(existingKnob);
     }
 
     const defaultValue = options.value;
@@ -34,7 +63,7 @@ export default class KnobManager {
     };
 
     knobStore.set(name, knobInfo);
-    return knobStore.get(name).value;
+    return this.getKnobValue(knobStore.get(name));
   }
 
   _mayCallChannel() {

addons/knobs/src/angular/index.js

@@ -1,5 +1,3 @@
-import addons from '@storybook/addons';
-
 import { prepareComponent } from './helpers';
 
 import {
@@ -15,7 +13,7 @@ import {
   selectV2,
   button,
   files,
-  manager,
+  makeDecorators,
 } from '../base';
 
 export { knob, text, boolean, number, color, object, array, date, select, selectV2, button, files };
@@ -23,19 +21,4 @@ export { knob, text, boolean, number, color, object, array, date, select, select
 export const angularHandler = (channel, knobStore) => getStory => context =>
   prepareComponent({ getStory, context, channel, knobStore });
 
-function wrapperKnobs(options) {
-  const channel = addons.getChannel();
-  manager.setChannel(channel);
-
-  if (options) channel.emit('addon:knobs:setOptions', options);
-
-  return angularHandler(channel, manager.knobStore);
-}
-
-export function withKnobs(storyFn, context) {
-  return wrapperKnobs()(storyFn)(context);
-}
-
-export function withKnobsOptions(options = {}) {
-  return (storyFn, context) => wrapperKnobs(options)(storyFn)(context);
-}
+export const { withKnobs, withKnobsOptions } = makeDecorators(angularHandler, { escapeHTML: true });

addons/knobs/src/base.js

@@ -1,4 +1,6 @@
 import deprecate from 'util-deprecate';
+import addons from '@storybook/addons';
+
 import KnobManager from './KnobManager';
 
 export const manager = new KnobManager();
@@ -73,3 +75,25 @@ export function button(name, callback, groupId) {
 export function files(name, accept, value = []) {
   return manager.knob(name, { type: 'files', accept, value });
 }
+
+export function makeDecorators(handler, defaultOptions = {}) {
+  function wrapperKnobs(options) {
+    const allOptions = { ...defaultOptions, ...options };
+
+    manager.setOptions(allOptions);
+    const channel = addons.getChannel();
+    manager.setChannel(channel);
+    channel.emit('addon:knobs:setOptions', allOptions);
+
+    return handler(channel, manager.knobStore);
+  }
+
+  return {
+    withKnobs(storyFn, context) {
+      return wrapperKnobs()(storyFn)(context);
+    },
+    withKnobsOptions(options = {}) {
+      return (storyFn, context) => wrapperKnobs(options)(storyFn)(context);
+    },
+  };
+}

addons/knobs/src/components/types/Button.js

@@ -35,7 +35,7 @@ ButtonType.propTypes = {
   onClick: PropTypes.func.isRequired,
 };
 
-ButtonType.serialize = value => value;
-ButtonType.deserialize = value => value;
+ButtonType.serialize = () => undefined;
+ButtonType.deserialize = () => undefined;
 
 export default ButtonType;

addons/knobs/src/mithril/index.js

@@ -2,7 +2,6 @@
 
 // eslint-disable-next-line import/no-extraneous-dependencies
 import m from 'mithril';
-import addons from '@storybook/addons';
 
 import WrapStory from './WrapStory';
 
@@ -18,7 +17,7 @@ import {
   select,
   selectV2,
   button,
-  manager,
+  makeDecorators,
 } from '../base';
 
 export { knob, text, boolean, number, color, object, array, date, select, selectV2, button };
@@ -31,19 +30,4 @@ export const mithrilHandler = (channel, knobStore) => getStory => context => {
   };
 };
 
-function wrapperKnobs(options) {
-  const channel = addons.getChannel();
-  manager.setChannel(channel);
-
-  if (options) channel.emit('addon:knobs:setOptions', options);
-
-  return mithrilHandler(channel, manager.knobStore);
-}
-
-export function withKnobs(storyFn, context) {
-  return wrapperKnobs()(storyFn)(context);
-}
-
-export function withKnobsOptions(options = {}) {
-  return (storyFn, context) => wrapperKnobs(options)(storyFn)(context);
-}
+export const { withKnobs, withKnobsOptions } = makeDecorators(mithrilHandler);

addons/knobs/src/polymer/index.js

@@ -1,4 +1,3 @@
-import addons from '@storybook/addons';
 import window from 'global';
 import './WrapStory.html';
 
@@ -14,6 +13,7 @@ import {
   select,
   files,
   manager,
+  makeDecorators,
 } from '../base';
 
 export { knob, text, boolean, number, color, object, array, date, select, files };
@@ -30,19 +30,4 @@ function prepareComponent({ getStory, context, channel, knobStore }) {
 export const polymerHandler = (channel, knobStore) => getStory => context =>
   prepareComponent({ getStory, context, channel, knobStore });
 
-function wrapperKnobs(options) {
-  const channel = addons.getChannel();
-  manager.setChannel(channel);
-
-  if (options) channel.emit('addon:knobs:setOptions', options);
-
-  return polymerHandler(channel, manager.knobStore);
-}
-
-export function withKnobs(storyFn, context) {
-  return wrapperKnobs()(storyFn)(context);
-}
-
-export function withKnobsOptions(options = {}) {
-  return (storyFn, context) => wrapperKnobs(options)(storyFn)(context);
-}
+export const { withKnobs, withKnobsOptions } = makeDecorators(polymerHandler, { escapeHTML: true });

addons/knobs/src/react/WrapStory.js

@@ -101,7 +101,7 @@ WrapStory.propTypes = {
     subscribe: PropTypes.func,
     unsubscribe: PropTypes.func,
   }).isRequired,
-  initialContent: PropTypes.object, // eslint-disable-line react/forbid-prop-types, react/no-unused-prop-types
+  initialContent: PropTypes.node, // eslint-disable-line react/no-unused-prop-types
 };
 
 polyfill(WrapStory);

addons/knobs/src/react/index.js

@@ -1,5 +1,4 @@
 import React from 'react';
-import addons from '@storybook/addons';
 
 import WrapStory from './WrapStory';
 
@@ -16,7 +15,7 @@ import {
   selectV2,
   button,
   files,
-  manager,
+  makeDecorators,
 } from '../base';
 
 export { knob, text, boolean, number, color, object, array, date, select, selectV2, button, files };
@@ -27,19 +26,4 @@ export const reactHandler = (channel, knobStore) => getStory => context => {
   return <WrapStory {...props} />;
 };
 
-function wrapperKnobs(options) {
-  const channel = addons.getChannel();
-  manager.setChannel(channel);
-
-  if (options) channel.emit('addon:knobs:setOptions', options);
-
-  return reactHandler(channel, manager.knobStore);
-}
-
-export function withKnobs(storyFn, context) {
-  return wrapperKnobs()(storyFn)(context);
-}
-
-export function withKnobsOptions(options = {}) {
-  return (storyFn, context) => wrapperKnobs(options)(storyFn)(context);
-}
+export const { withKnobs, withKnobsOptions } = makeDecorators(reactHandler);

addons/knobs/src/vue/index.js

@@ -1,5 +1,3 @@
-import addons from '@storybook/addons';
-
 import {
   knob,
   text,
@@ -13,7 +11,7 @@ import {
   selectV2,
   button,
   files,
-  manager,
+  makeDecorators,
 } from '../base';
 
 export { knob, text, boolean, number, color, object, array, date, select, selectV2, button, files };
@@ -74,19 +72,4 @@ export const vueHandler = (channel, knobStore) => getStory => context => ({
   },
 });
 
-function wrapperKnobs(options) {
-  const channel = addons.getChannel();
-  manager.setChannel(channel);
-
-  if (options) channel.emit('addon:knobs:setOptions', options);
-
-  return vueHandler(channel, manager.knobStore);
-}
-
-export function withKnobs(storyFn, context) {
-  return wrapperKnobs()(storyFn)(context);
-}
-
-export function withKnobsOptions(options = {}) {
-  return (storyFn, context) => wrapperKnobs(options)(storyFn)(context);
-}
+export const { withKnobs, withKnobsOptions } = makeDecorators(vueHandler, { escapeHTML: true });

examples/angular-cli/src/stories/__snapshots__/addon-knobs.stories.storyshot

@@ -120,3 +120,15 @@ exports[`Storyshots Addon|Knobs Simple 1`] = `
   </ng-component>
 </storybook-dynamic-app-root>
 `;
+
+exports[`Storyshots Addon|Knobs XSS safety 1`] = `
+<storybook-dynamic-app-root
+  cfr={[Function CodegenComponentFactoryResolver]}
+  data={[Function Object]}
+  target={[Function ViewContainerRef_]}
+>
+  <ng-component>
+    &lt;img src=x onerror="alert('XSS Attack')" &gt;
+  </ng-component>
+</storybook-dynamic-app-root>
+`;

examples/angular-cli/src/stories/addon-knobs.stories.ts

@@ -79,4 +79,7 @@ storiesOf('Addon|Knobs', module)
         nice,
       },
     };
-  });
+  })
+  .add('XSS safety', () => ({
+    template: text('Rendered string', '<img src=x onerror="alert(\'XSS Attack\')" >'),
+  }));

examples/mithril-kitchen-sink/src/stories/addon-knobs.stories.js

@@ -68,4 +68,7 @@ storiesOf('Addons|Knobs', module)
         </div>
       ),
     };
-  });
+  })
+  .add('XSS safety', () => ({
+    view: () => text('Rendered string', '<img src=x onerror="alert(\'XSS Attack\')" >'),
+  }));

examples/official-storybook/stories/__snapshots__/addon-knobs.stories.storyshot

@@ -1,5 +1,11 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`Storyshots Addons|Knobs.withKnobs XSS safety 1`] = `
+<div>
+  &lt;img src=x onerror="alert('XSS Attack')" &gt;
+</div>
+`;
+
 exports[`Storyshots Addons|Knobs.withKnobs dynamic knobs 1`] = `
 <div>
   <div>

examples/official-storybook/stories/addon-knobs.stories.js

@@ -189,6 +189,9 @@ storiesOf('Addons|Knobs.withKnobs', module)
       <p>Hit the knob load button and it should trigger an async load after a short delay</p>
       <AsyncItemLoader />
     </div>
+  ))
+  .add('XSS safety', () => (
+    <div>{text('Rendered string', '<img src=x onerror="alert(\'XSS Attack\')" >')}</div>
   ));
 
 storiesOf('Addons|Knobs.withKnobsOptions', module)

examples/polymer-cli/src/stories/addon-knobs.stories.js

@@ -58,4 +58,5 @@ storiesOf('Addon|Knobs', module)
             <p>${nice ? 'Nice to meet you!' : 'Leave me alone!'}</p>
           </div>
         `;
-  });
+  })
+  .add('XSS safety', () => text('Rendered string', '<img src=x onerror="alert(\'XSS Attack\')" >'));

examples/vue-kitchen-sink/src/stories/__snapshots__/addon-knobs.stories.storyshot

@@ -43,3 +43,11 @@ exports[`Storyshots Addon|Knobs Simple 1`] = `
   I am John Doe and I'm 44 years old.
 </div>
 `;
+
+exports[`Storyshots Addon|Knobs XSS safety 1`] = `
+<div>
+  
+        &lt;img src=x onerror="alert('XSS Attack')" &gt;
+      
+</div>
+`;

examples/vue-kitchen-sink/src/stories/addon-knobs.stories.js

@@ -66,4 +66,11 @@ storiesOf('Addon|Knobs', module)
           </div>
         `,
     };
-  });
+  })
+  .add('XSS safety', () => ({
+    template: `
+      <div>
+        ${text('Rendered string', '<img src=x onerror="alert(\'XSS Attack\')" >')}
+      </div>
+    `,
+  }));

yarn.lock

@@ -5357,7 +5357,7 @@ escape-html@1.0.2:
   version "1.0.2"
   resolved "https://registry.yarnpkg.com/escape-html/-/escape-html-1.0.2.tgz#d77d32fa98e38c2f41ae85e9278e0e0e6ba1022c"
 
-escape-html@~1.0.3:
+escape-html@^1.0.3, escape-html@~1.0.3:
   version "1.0.3"
   resolved "https://registry.yarnpkg.com/escape-html/-/escape-html-1.0.3.tgz#0258eae4d3d0c0974de1c169188ef0051d1d1988"