Validate assets against file map before serving (#1643)

Summary:
Pull Request resolved: #1643

Assets served via the `/assets/` endpoint were not being validated against Metro's file map. This meant that files excluded from the file map (via `blockList`, `.git`/`.hg` directories, etc.) could still be accessed through the asset serving endpoint, potentially exposing sensitive files.

This change adds file map validation to `getAsset()` by accepting an optional file existence check function. The Server now passes the DependencyGraph's `doesFileExist` method, which checks whether a file is present in the file map. Assets not in the file map are rejected with an appropriate error message.

This approach is more robust than checking blockList directly because the file map already applies all filtering logic (blockList patterns, VCS directories like `.git`/`.hg`, etc.), ensuring assets follow the same visibility rules as modules.

Reviewed By: robhogan

Differential Revision: D91128421

fbshipit-source-id: 0721599a609a54f02b2ff9f1474fe33726b8ee5e

Author: fabriziocucci

docs/Configuration.md

@@ -302,7 +302,7 @@ If set to `false`, prevents Metro from using Watchman (even if it's installed).
 
 Type: `RegExp` or `Array<RegExp>`
 
-A regular expression (or list of regular expressions) defining which paths to exclude from Metro's file map. Files whose absolute paths match these patterns are effectively hidden from Metro and cannot be resolved or imported in the current project.
+A regular expression (or list of regular expressions) defining which paths to exclude from Metro's file map. Files whose absolute paths match these patterns are effectively hidden from Metro and cannot be resolved or imported in the current project. Additionally, blocked files cannot be served via the `/assets/` endpoint.
 
 #### `hasteImplModulePath`
 

packages/metro/src/Assets.js

@@ -282,6 +282,7 @@ export async function getAsset(
   watchFolders: ReadonlyArray<string>,
   platform: ?string = null,
   assetExts: ReadonlyArray<string>,
+  fileExistsInFileMap?: (absolutePath: string) => boolean,
 ): Promise<Buffer> {
   const assetData = AssetPaths.parse(
     relativePath,
@@ -296,10 +297,19 @@ export async function getAsset(
     );
   }
 
-  if (!pathBelongsToRoots(absolutePath, [projectRoot, ...watchFolders])) {
-    throw new Error(
-      `'${relativePath}' could not be found, because it cannot be found in the project root or any watch folder`,
-    );
+  // NOTE: If fileExistsInFileMap is not provided, we fall back to pathBelongsToRoots for backward compatibility, as getAsset is part of the public API.
+  if (fileExistsInFileMap != null) {
+    if (!fileExistsInFileMap(absolutePath)) {
+      throw new Error(
+        `'${relativePath}' could not be found, because it is not within the projectRoot or watchFolders, or it is blocked via the resolver.blockList config`,
+      );
+    }
+  } else {
+    if (!pathBelongsToRoots(absolutePath, [projectRoot, ...watchFolders])) {
+      throw new Error(
+        `'${relativePath}' could not be found, because it cannot be found in the project root or any watch folder`,
+      );
+    }
   }
 
   const record = await getAbsoluteAssetRecord(absolutePath, platform);

packages/metro/src/Server.js

@@ -549,12 +549,14 @@ export default class Server {
     );
 
     try {
+      const depGraph = await this._bundler.getBundler().getDependencyGraph();
       const data = await getAsset(
         assetPath,
         this._config.projectRoot,
         this._config.watchFolders,
         urlObj.searchParams.get('platform'),
         this._config.resolver.assetExts,
+        filePath => depGraph.doesFileExist(filePath),
       );
       // Tell clients to cache this for 1 year.
       // This is safe as the asset url contains a hash of the asset.

packages/metro/src/Server/__tests__/Server-test.js

@@ -839,6 +839,7 @@ describe('processRequest', () => {
         ['/root'],
         'ios',
         expect.any(Array),
+        expect.any(Function),
       );
     });
 
@@ -856,6 +857,7 @@ describe('processRequest', () => {
         ['/root'],
         'ios',
         expect.any(Array),
+        expect.any(Function),
       );
       expect(response._getString()).toBe(mockData.slice(0, 4));
     });
@@ -912,6 +914,7 @@ describe('processRequest', () => {
         ['/root'],
         null,
         expect.any(Array),
+        expect.any(Function),
       );
     });
 
@@ -936,6 +939,7 @@ describe('processRequest', () => {
         ['/root'],
         'ios',
         expect.any(Array),
+        expect.any(Function),
       );
       expect(response._getString()).toBe('i am image');
     });
@@ -953,6 +957,7 @@ describe('processRequest', () => {
         ['/root'],
         null,
         expect.any(Array),
+        expect.any(Function),
       );
       expect(response._getString()).toBe('i am image');
     });

packages/metro/src/__tests__/Assets-test.js

@@ -150,6 +150,22 @@ describe('getAsset', () => {
       getAssetStr('../anotherfolder/b.png', '/root', [], null, ['png']),
     ).rejects.toBeInstanceOf(Error);
   });
+
+  test('should find an image when fileExistsInFileMap returns true', async () => {
+    writeImages({'b.png': 'b image'});
+
+    expect(
+      await getAssetStr('imgs/b.png', '/root', [], null, ['png'], () => true),
+    ).toBe('b image');
+  });
+
+  test('should throw an error when fileExistsInFileMap returns false', async () => {
+    writeImages({'b.png': 'b image'});
+
+    await expect(
+      getAssetStr('imgs/b.png', '/root', [], null, ['png'], () => false),
+    ).rejects.toBeInstanceOf(Error);
+  });
 });
 
 describe('getAssetData', () => {

packages/metro/src/node-haste/DependencyGraph.js

@@ -178,7 +178,7 @@ export default class DependencyGraph extends EventEmitter {
       },
       disableHierarchicalLookup:
         this._config.resolver.disableHierarchicalLookup,
-      doesFileExist: this._doesFileExist,
+      doesFileExist: this.doesFileExist,
       emptyModulePath: this._config.resolver.emptyModulePath,
       extraNodeModules: this._config.resolver.extraNodeModules,
       fileSystemLookup,
@@ -363,7 +363,7 @@ export default class DependencyGraph extends EventEmitter {
     return resolution;
   }
 
-  _doesFileExist = (filePath: string): boolean => {
+  doesFileExist = (filePath: string): boolean => {
     return this._fileSystem.exists(filePath);
   };