Merge pull request #38 from Shopify/address-security-issue

Address security concern
Shopify · Jan 27, 2020 · 74199b8 · 74199b8
2 parents 0dd7437 + a88c6e0
commit 74199b8
Show file tree

Hide file tree

Showing 11 changed files with 77 additions and 44 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1,10 @@
+# Change Log
+
+## v1.0.1 (Jan 27, 2019)
+
+* [#37](https://github.com/Shopify/shopify-theme-inspector/pull/37) Fix custom domain redirect
+* [#38](https://github.com/Shopify/shopify-theme-inspector/pull/38) Address security concern
+
+## v1.0.0 (Jan. 14, 2019)
+
+* First release of the extension
diff --git a/src/components/liquid-flamegraph.ts b/src/components/liquid-flamegraph.ts
@@ -2,7 +2,12 @@ import * as d3 from 'd3';
 import * as flamegraph from 'd3-flame-graph';
 import 'd3-flame-graph/dist/d3-flamegraph.css';
 import {debounce} from 'lodash';
-import {formatNodeTime, getThemeId} from '../utils';
+import {
+  formatNodeTime,
+  getThemeId,
+  emptyHTMLNode,
+  updateInfoText,
+} from '../utils';
 
 const selectors = {
   partial: '[data-partial]',
@@ -33,7 +38,7 @@ export default class LiquidFlamegraph {
   }
 
   display(): void {
-    this.element.innerHTML = '';
+    emptyHTMLNode(this.element);
     d3.select(this.element)
       .datum(this.profile)
       .call(this.flamegraph);
@@ -63,26 +68,19 @@ export default class LiquidFlamegraph {
   }
 
   async displayNodeDetails(node: FlamegraphNode) {
-    document.querySelector(
-      selectors.partial,
-    )!.innerHTML = `File: ${node.data.name}`;
-
-    document.querySelector(
-      selectors.nodeTime,
-    )!.innerHTML = `Total Time: <b>${formatNodeTime(node.value)}ms</b>`;
+    updateInfoText(selectors.partial, node.data.name);
+    updateInfoText(selectors.nodeTime, `${formatNodeTime(node.value)}ms`);
 
     const clickableLink = await this.generateClickableLink(
       node.data.name,
       node.data.line,
     );
 
-    document.querySelector(
-      selectors.code,
-    )!.innerHTML = `Code snippet: <a href="${clickableLink}" target="_blank"><i><span class="code-snippet">${node.data.code}</span></i></a>`;
+    const codeLink = document.querySelector(selectors.code);
+    codeLink!.querySelector('a')!.href = clickableLink;
+    codeLink!.querySelector('.code-snippet')!.textContent = node.data.code;
 
-    document.querySelector(
-      selectors.line,
-    )!.innerHTML = `Line: ${node.data.line}`;
+    updateInfoText(selectors.line, `${node.data.line}`);
   }
 
   async generateClickableLink(

diff --git a/src/devtools.html b/src/devtools.html
@@ -43,15 +43,17 @@ <h2>This page cannot be profiled</h2>
     <p>Click <a href="https://github.com/Shopify/shopify-devtools#extension-usage" target="_blank">here</a> to read more about this.</p>
   </div>
 
-  <div data-flamegraph-wrapper>
+  <div data-flamegraph-wrapper class="hide">
     <div data-flamegraph-container class="chart"></div>
-    <p data-total-time class="total-time"></p>
+    <p data-total-time class="total-time">Total time to render liquid: <b>-</b></p>
 
     <div data-detailed-info class="detailed-info">
-      <p data-partial></p>
-      <p data-node-time></p>
-      <p data-code></p>
-      <p data-line></p>
+      <p data-partial>File: <b>-</b></p>
+      <p data-node-time>Total Time: <b>-</b></p>
+      <p data-code>
+        Code snippet: <a href="" target="_blank"><i><span class="code-snippet"></span></i></a>
+      </p>
+      <p data-line>Line: <b>-</b></p>
     </div>
   </div>
 </body>

diff --git a/src/devtools.ts b/src/devtools.ts
@@ -1,6 +1,11 @@
 import Toolbar from './components/toolbar';
 import LiquidFlamegraph from './components/liquid-flamegraph';
-import {getProfileData, setTotalTime, getBrowserTheme} from './utils';
+import {
+  getProfileData,
+  setTotalTime,
+  getBrowserTheme,
+  emptyHTMLNode,
+} from './utils';
 
 import './styles/devtools.css';
 
@@ -43,7 +48,7 @@ function getInspectedWindowURL(): Promise<URL> {
 }
 
 async function refreshPanel() {
-  document.querySelector(selectors.initialMessage)!.innerHTML = '';
+  emptyHTMLNode(document.querySelector(selectors.initialMessage));
   document
     .querySelector(selectors.flamegraphWrapper)!
     .classList.add('loading-fade');

diff --git a/src/popup.ts b/src/popup.ts
@@ -14,15 +14,14 @@ const signOutButton = document.querySelector(`[data-sign-out]`);
 
 async function setSignedInPopup() {
   const popupSignedInPrompt = document.querySelector(
-    selectors.popupSignedInPrompt,
+    `${selectors.popupSignedInPrompt} b`,
   );
 
   if (popupSignedIn) popupSignedIn.classList.remove('hide');
   if (popupSignIn) popupSignIn.classList.add('hide');
 
   const name = await getUserName();
-  if (popupSignedInPrompt)
-    popupSignedInPrompt.innerHTML = `You are logged in as <b>${name}</b>`;
+  popupSignedInPrompt!.textContent = `${name}`;
 }
 
 function setSignInPopup() {
@@ -70,8 +69,8 @@ if (signInButton) {
         }
       },
     );
-    signInButton.innerHTML =
-      '<div data-loading-animation class="loader" style="display: inline-block"></div>';
+    signInButton.querySelector('span')?.classList.add('hide');
+    signInButton.querySelector('.loader')?.classList.remove('hide');
   });
 }
 
@@ -85,8 +84,8 @@ if (signOutButton) {
         setSignInPopup();
       }
     });
-    signOutButton.innerHTML =
-      '<div data-loading-animation class="loader" style="display: inline-block"></div>';
+    signOutButton.querySelector('span')?.classList.add('hide');
+    signOutButton.querySelector('.loader')?.classList.remove('hide');
   });
 }
 

diff --git a/src/popupAuthFlow.html b/src/popupAuthFlow.html
@@ -7,15 +7,21 @@
       <img class="shopify-logo" src="./images/shopify_logo_black.png" alt="Shopify Logo">
       <hr class="faded-hr">
       <p class="login-prompt">Sign in to your Shopify account</p>
-      <button class="popup-button" id="signInButton" data-sign-in>Sign In</button>
+      <button class="popup-button" id="signInButton" data-sign-in>
+        <span>Sign In</span>
+        <div data-loading-animation class="loader hide" style="display: inline-block"></div>
+      </button>
     </div>
 
     <div class="popup hide" data-popup-signed-in>
       <img class="shopify-logo" src="./images/shopify_logo_black.png" alt="Shopify Logo">
       <hr class="faded-hr">
-      <p class="login-prompt" data-signed-in-prompt>&nbsp;</p>
+      <p class="login-prompt" data-signed-in-prompt>You are logged in as <b>-</b></p>
       <p class="login-prompt"><a href="https://developers.google.com/web/tools/chrome-devtools/open" target="_blank">Open Chrome DevTools</a> and select the <b>Shopify</b> tab</p>
-      <button class="popup-button" id="signOutButton" data-sign-out>Sign Out</button>
+      <button class="popup-button" id="signOutButton" data-sign-out>
+        <span>Sign Out</span>
+        <div data-loading-animation class="loader hide" style="display: inline-block"></div>
+      </button>
     </div>
   </body>
 </html>
diff --git a/src/styles/popup.css b/src/styles/popup.css
@@ -33,7 +33,7 @@
 }
 
 .hide {
-  display: none;
+  display: none !important;
 }
 
 .loader {

diff --git a/src/utils/domHelpers.ts b/src/utils/domHelpers.ts
@@ -1,9 +1,5 @@
 export function setTotalTime(totalTime: number) {
-  document.querySelector(
-    '[data-total-time]',
-  )!.innerHTML = `Total time to render liquid: <b>${Math.trunc(
-    totalTime * 1000,
-  )}ms</b>`;
+  updateInfoText('[data-total-time]', `${Math.trunc(totalTime * 1000)}ms`);
 }
 
 export function formatNodeTime(nodeTime: number) {
@@ -14,3 +10,15 @@ export function formatNodeTime(nodeTime: number) {
     return '< 1';
   }
 }
+
+export function emptyHTMLNode(node: any) {
+  if (!node) return;
+
+  while (node.firstChild) {
+    node.removeChild(node.firstChild);
+  }
+}
+
+export function updateInfoText(selector: string, updateText: string) {
+  document.querySelector(`${selector} b`)!.textContent = updateText;
+}
diff --git a/src/utils/getProfileData.ts b/src/utils/getProfileData.ts
@@ -27,7 +27,7 @@ export async function getProfileData(
   }
 
   const profileData = JSON.parse(
-    nullthrows(document.querySelector('#liquidProfileData')).innerHTML,
+    nullthrows(document.querySelector('#liquidProfileData')).textContent || '',
   );
 
   return cleanProfileData(profileData);

diff --git a/src/utils/index.ts b/src/utils/index.ts
@@ -1,6 +1,11 @@
 export {getProfileData} from './getProfileData';
 export {isDev, getThemeId, getBrowserTheme} from './helpers';
-export {setTotalTime, formatNodeTime} from './domHelpers';
+export {
+  setTotalTime,
+  formatNodeTime,
+  emptyHTMLNode,
+  updateInfoText,
+} from './domHelpers';
 export {Oauth2} from './oauth2';
 export {
   saveToLocalStorage,

diff --git a/test/devtools.test.ts b/test/devtools.test.ts
@@ -15,7 +15,7 @@ describe('Devtools', () => {
   });
 
   it('test initial message for devtools window is displayed', async () => {
-    const elementText = await page.$eval('.initial', elem => elem.innerHTML);
+    const elementText = await page.$eval('.initial', elem => elem.textContent);
     expect(elementText).toStrictEqual('No profiling recorded yet');
   });
 
@@ -24,7 +24,7 @@ describe('Devtools', () => {
     await page.$eval('[data-refresh-button]', elem => elem.click());
     const flamegraphElement = await page.$eval(
       '.d3-flame-graph',
-      elem => elem.innerHTML,
+      elem => elem.textContent,
     );
     expect(flamegraphElement).not.toBeNull();
   });
@@ -50,7 +50,7 @@ describe('Devtools', () => {
     await page.$eval('.d3-flame-graph-label', elem => elem.click());
     const elementHtml = await page.$eval(
       '[data-detailed-info]',
-      elem => elem.innerHTML,
+      elem => elem.textContent,
     );
     expect(elementHtml).not.toBeNull();
   });
-Original file line number
+Diff line change
@@ Expand Up / @@ -33,7 +33,7 @@ @@
     }
     .hide {
-      display: none;
+      display: none !important;
     }
     .loader {
@@ Expand Down @@