Show warning when using Math.random (#950)

* Return warnings from post processing and add show warning on Math.random

* Show warning in bundler plugins

packages/snaps-browserify-plugin/src/plugin.test.ts

@@ -10,7 +10,11 @@ import {
   DEFAULT_SNAP_BUNDLE,
   getSnapManifest,
 } from '@metamask/snaps-utils/test-utils';
-import { checkManifest, evalBundle } from '@metamask/snaps-utils';
+import {
+  checkManifest,
+  evalBundle,
+  PostProcessWarning,
+} from '@metamask/snaps-utils';
 import plugin, { Options, SnapsBrowserifyTransform } from './plugin';
 
 jest.mock('fs');
@@ -129,6 +133,18 @@ describe('plugin', () => {
     expect(result).toMatchSnapshot();
   });
 
+  it('logs post processing warnings', async () => {
+    jest.spyOn(console, 'log').mockImplementation(() => undefined);
+
+    await bundle({
+      code: 'console.log(Math.random());',
+    });
+
+    expect(console.log).toHaveBeenCalledWith(
+      `Bundle Warning: Processing of the Snap bundle completed with warnings.\n${PostProcessWarning.UnsafeMathRandom}`,
+    );
+  });
+
   it('generates a source map', async () => {
     const result = await bundle({ browserifyOptions: { debug: true } });
 

packages/snaps-browserify-plugin/src/plugin.ts

@@ -125,6 +125,14 @@ export class SnapsBrowserifyTransform extends Transform {
       inputSourceMap,
     });
 
+    if (result.warnings.length > 0) {
+      console.log(
+        `Bundle Warning: Processing of the Snap bundle completed with warnings.\n${result.warnings.join(
+          '\n',
+        )}`,
+      );
+    }
+
     postBundle(this.#options, result.code)
       .catch((error) => {
         callback(error);

packages/snaps-rollup-plugin/src/plugin.test.ts

@@ -5,7 +5,11 @@ import {
   DEFAULT_SNAP_BUNDLE,
   getSnapManifest,
 } from '@metamask/snaps-utils/test-utils';
-import { checkManifest, evalBundle } from '@metamask/snaps-utils';
+import {
+  checkManifest,
+  evalBundle,
+  PostProcessWarning,
+} from '@metamask/snaps-utils';
 import snaps, { Options } from './plugin';
 
 jest.mock('fs');
@@ -143,6 +147,18 @@ describe('snaps', () => {
     `);
   });
 
+  it('logs post processing warnings', async () => {
+    jest.spyOn(console, 'log').mockImplementation(() => undefined);
+
+    await bundle({
+      code: 'console.log(Math.random());',
+    });
+
+    expect(console.log).toHaveBeenCalledWith(
+      `Bundle Warning: Processing of the Snap bundle completed with warnings.\n${PostProcessWarning.UnsafeMathRandom}`,
+    );
+  });
+
   it('generates a source map', async () => {
     await fs.writeFile('/source-map.ts', DEFAULT_SNAP_BUNDLE);
 

packages/snaps-rollup-plugin/src/plugin.ts

@@ -50,6 +50,14 @@ export default function snaps(options?: Partial<Options>): Plugin {
         sourceMap: true,
       });
 
+      if (result.warnings.length > 0) {
+        this.warn(
+          `Bundle Warning: Processing of the Snap bundle completed with warnings.\n${result.warnings.join(
+            '\n',
+          )}`,
+        );
+      }
+
       return { code: result.code, map: result.sourceMap };
     },
 

packages/snaps-utils/src/post-process.test.ts

@@ -1,4 +1,4 @@
-import { postProcessBundle } from './post-process';
+import { postProcessBundle, PostProcessWarning } from './post-process';
 
 describe('postProcessBundle', () => {
   it('trims the string', () => {
@@ -58,6 +58,7 @@ describe('postProcessBundle', () => {
       (1, eval)("<!" + "--" + " foo " + "--" + ">");
       (1, foo.eval)("<!" + "--" + " bar " + "--" + ">");",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -84,6 +85,7 @@ describe('postProcessBundle', () => {
         const bar = 'baz';
       }",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -112,6 +114,7 @@ describe('postProcessBundle', () => {
         regeneratorRuntime.bar();
       }",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -124,6 +127,7 @@ describe('postProcessBundle', () => {
 
       var _marked = [a].map(regeneratorRuntime.mark);",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -138,6 +142,7 @@ describe('postProcessBundle', () => {
       {
         "code": "const foo = 'regeneratorRuntime';",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -152,6 +157,7 @@ describe('postProcessBundle', () => {
       {
         "code": "const foo = "<!" + "--" + " bar " + "--" + ">";",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -168,6 +174,7 @@ describe('postProcessBundle', () => {
         "code": "const foo = "foo bar " + "import" + "()" + " baz";
       const bar = "foo bar " + "import" + "(this works too)" + " baz";",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -182,6 +189,7 @@ describe('postProcessBundle', () => {
       {
         "code": "const foo = \`\${"<!"}\${"--"} bar \${"--"}\${">"} \${"<!" + "--" + " baz " + "--" + ">"} \${qux}\`;",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -203,6 +211,7 @@ describe('postProcessBundle', () => {
       foo\`
               foo \${"import"}\${"()"} \${"import" + "(bar)"} \${qux}      \`;",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -217,6 +226,7 @@ describe('postProcessBundle', () => {
       {
         "code": "// < !-- foo -- >",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -233,6 +243,7 @@ describe('postProcessBundle', () => {
         "code": "// Foo bar import\\() baz
       // Foo bar import\\(baz) qux",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -247,6 +258,7 @@ describe('postProcessBundle', () => {
       {
         "code": "const foo = '';",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -265,6 +277,7 @@ describe('postProcessBundle', () => {
       const foo = 'foo';
       const bar = \`bar\${foo}\`;",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -278,6 +291,7 @@ describe('postProcessBundle', () => {
       {
         "code": "const foo = \`\${"<!"}\${"--"} \\\` \${foo} \\\` \${"--"}\${">"}\`;",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -320,6 +334,7 @@ describe('postProcessBundle', () => {
         (1, foo.eval)('bar');
       });",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -362,6 +377,7 @@ describe('postProcessBundle', () => {
           ],
           "version": 3,
         },
+        "warnings": [],
       }
     `);
   });
@@ -380,6 +396,7 @@ describe('postProcessBundle', () => {
         "code": "const foo = 'bar';
       //# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJuYW1lcyI6WyJmb28iXSwic291cmNlcyI6WyJ1bmtub3duIl0sInNvdXJjZXNDb250ZW50IjpbIlxuICAgICAgY29uc3QgZm9vID0gJ2Jhcic7XG4gICAgIl0sIm1hcHBpbmdzIjoiQUFDTSxNQUFNQSxHQUFHLEdBQUcsS0FBWiJ9",
         "sourceMap": null,
+        "warnings": [],
       }
     `);
   });
@@ -432,7 +449,18 @@ describe('postProcessBundle', () => {
           ],
           "version": 3,
         },
+        "warnings": [],
       }
     `);
   });
+
+  it('returns a warning when using Math.random', () => {
+    const code = `
+      const foo = Math.random();
+    `;
+
+    const { warnings } = postProcessBundle(code);
+    expect(warnings).toHaveLength(1);
+    expect(warnings[0]).toBe(PostProcessWarning.UnsafeMathRandom);
+  });
 });

packages/snaps-utils/src/post-process.ts

@@ -47,12 +47,18 @@ export type PostProcessOptions = {
  * @property code - The modified code.
  * @property sourceMap - The source map for the modified code, if the source map
  * option was enabled.
+ * @property warnings - Any warnings that occurred during the post-processing.
  */
 export type PostProcessedBundle = {
   code: string;
   sourceMap?: SourceMap | null;
+  warnings: PostProcessWarning[];
 };
 
+export enum PostProcessWarning {
+  UnsafeMathRandom = '`Math.random` was detected in the bundle. This is not a secure source of randomness.',
+}
+
 // The RegEx below consists of multiple groups joined by a boolean OR.
 // Each part consists of two groups which capture a part of each string
 // which needs to be split up, e.g., `<!--` is split into `<!` and `--`.
@@ -209,6 +215,8 @@ export function postProcessBundle(
     inputSourceMap,
   }: Partial<PostProcessOptions> = {},
 ): PostProcessedBundle {
+  const warnings = new Set<PostProcessWarning>();
+
   const pre: PluginObj['pre'] = ({ ast }) => {
     ast.comments?.forEach((comment) => {
       // Break up tokens that could be parsed as HTML comment terminators. The
@@ -255,6 +263,17 @@ export function postProcessBundle(
           }),
         );
       }
+
+      // Detect the use of `Math.random()` and add a warning.
+      if (
+        node.callee.type === 'MemberExpression' &&
+        node.callee.object.type === 'Identifier' &&
+        node.callee.object.name === 'Math' &&
+        node.callee.property.type === 'Identifier' &&
+        node.callee.property.name === 'random'
+      ) {
+        warnings.add(PostProcessWarning.UnsafeMathRandom);
+      }
     },
 
     MemberExpression(path) {
@@ -437,7 +456,11 @@ export function postProcessBundle(
       throw new Error('Bundled code is empty.');
     }
 
-    return { code: file.code, sourceMap: file.map };
+    return {
+      code: file.code,
+      sourceMap: file.map,
+      warnings: Array.from(warnings),
+    };
   } catch (error) {
     throw new Error(`Failed to post process code:\n${error.message}`);
   }

packages/snaps-webpack-plugin/src/plugin.test.ts

@@ -8,7 +8,11 @@ import {
   DEFAULT_SNAP_BUNDLE,
   getSnapManifest,
 } from '@metamask/snaps-utils/test-utils';
-import { checkManifest, evalBundle } from '@metamask/snaps-utils';
+import {
+  checkManifest,
+  evalBundle,
+  PostProcessWarning,
+} from '@metamask/snaps-utils';
 import SnapsWebpackPlugin, { Options } from './plugin';
 
 jest.mock('@metamask/snaps-utils', () => ({
@@ -139,6 +143,16 @@ describe('SnapsWebpackPlugin', () => {
     expect(code).not.toContain(`// Returns baz`);
   });
 
+  it('logs post processing warnings', async () => {
+    const { stats } = await bundle({
+      code: 'console.log(Math.random());',
+    });
+
+    expect(stats.toJson().warnings?.[0].message).toMatch(
+      `SnapsWebpackPlugin: Bundle Warning: Processing of the Snap bundle completed with warnings.\n${PostProcessWarning.UnsafeMathRandom}`,
+    );
+  });
+
   it('generates a source map', async () => {
     const { fs } = await bundle({
       webpackOptions: {

packages/snaps-webpack-plugin/src/plugin.ts

@@ -67,6 +67,16 @@ export default class SnapsWebpackPlugin {
             inputSourceMap: devtool ? (asset.map() as SourceMap) : undefined,
           });
 
+          if (processed.warnings.length > 0) {
+            compilation.warnings.push(
+              new WebpackError(
+                `${PLUGIN_NAME}: Bundle Warning: Processing of the Snap bundle completed with warnings.\n${processed.warnings.join(
+                  '\n',
+                )}`,
+              ),
+            );
+          }
+
           const replacement = processed.sourceMap
             ? new SourceMapSource(
                 processed.code,