[flutter_tools] Contain web asset server path resolution to its search roots by r0g18 · Pull Request #194064 · flutter/flutter

r0g18 · 2026-10-09T07:36:29Z

flutter_tools' web asset servers resolve the incoming request path against each of their search roots with Uri.resolve. That treats the request path as a URI reference rather than as a path, and a reference carrying a scheme, an authority or an absolute path replaces the base instead of resolving below it. The result of a lookup is therefore not guaranteed to be inside the root that was being searched, and nothing downstream checks that it is.

This PR makes the containment explicit instead of relying on the shape of the request path.

Two helpers are added to web_server_utilities.dart:

isRelativeRequestPath rejects a request path that is not a relative reference: one with a scheme or a Windows drive letter in its first segment, an authority, an absolute path, a backslash, or a NUL.
resolveRequestPathUnder resolves against a given root and returns the result only when it lands inside that root, compared with path.isWithin so the comparison is per-component and case-insensitive on Windows.

Every resolve() call in ReleaseAssetServer.handle and WebAssetServer.handleRequest now goes through the helper, so each search root keeps its own boundary: the canvaskit fallback, the built asset fallback, the project web directory, and the roots consulted by _resolveDartFile. . and .. segments are deliberately still accepted and handled by the containment check, so a request path that normalizes back inside the root keeps working. A /packages/<package>/<path> request is resolved the same way below that package's packageUriRoot, looked up by name, instead of through a package: URI built from the request segments. ReleaseAssetServer checks a root's allowed extensions against the request path before resolving it, so regular asset requests skip resolution for the source-map-only roots.

This also keeps exceptions from escaping the handler. On master today, GET /..%2F..%2F..%2Ftmp/foo throws UnsupportedError: Illegal path character out of toFilePath, because the decoded segment contains a separator and so names no file path at all. Likewise GET /packages/foo.dart throws an ArgumentError out of PackageConfig.resolve. Both are now answered like any other path that does not resolve inside a root.

Tests

ReleaseAssetServer does not serve files outside of the project is extended with the reference shapes that resolve outside a search root, and WebAssetServer does not serve files outside of the project is added as the debug-handler equivalent, including dartSourceContents and /packages/ requests with an absolute path or nothing after the package name. Both fail against unmodified lib/ and pass with the change. Positive tests cover a package path with a dot segment that stays inside the package root, and nested build output paths, including an asset name containing a colon in a later segment, which stays valid in a relative reference and must keep working.

dart analyze and dart format are clean on the five touched files. Rebased on current master; locally both web suites (devfs_web_test.dart, web_asset_server_test.dart) pass in full, 84 tests. The 4 failures I mentioned in an earlier version of this description came from running them without FLUTTER_ROOT set.

Issues

There is no public issue tracking this; it came out of maintainer feedback rather than a filed report. Happy to open one if the process needs the link.

Pre-launch Checklist

gemini-code-assist

Code Review

This pull request introduces path traversal and absolute URI resolution protections in ReleaseAssetServer and WebAssetServer by implementing isRelativeRequestPath and resolveRequestPathUnder helper functions. These helpers ensure that untrusted request paths are relative and resolve strictly within the designated trusted root directories, preventing access to files outside the project. Additionally, comprehensive unit tests are added to verify these security boundaries on both POSIX and Windows systems. I have no feedback to provide as there are no review comments.

mdebbar

Thanks for the contribution!

This looks good to me. I just have some minor suggestions to improve the comments and tests.

mdebbar · 2026-10-23T15:03:45Z

+  //
+  // An empty set relies on the request path staying below its root, which
+  // `resolveRequestPathUnder` enforces for every root listed below.


This reads as part of the _sourceMapExtensions comment, but it's about the empty set in _searchPaths. Let's move it there:

Suggested change

//

// An empty set relies on the request path staying below its root, which

// `resolveRequestPathUnder` enforces for every root listed below.

Done, moved it to the _searchPaths comment where the empty set is described.

mdebbar · 2026-10-23T15:03:45Z

+      // A dot segment would resolve outside the package root, which package
+      // resolution does not guard against on its own.


I don't think a dot segment can get outside the package root here: Uri collapses them, and once they climb past the package name PackageConfig.resolve throws instead.

Suggested change

// A dot segment would resolve outside the package root, which package

// resolution does not guard against on its own.

// A dot segment that climbs past the package name makes

// `PackageConfig.resolve` throw.

You're right, thanks. Applied your wording, and updated the commit message and PR description to match.

mdebbar · 2026-10-23T15:03:45Z

+      '../secret.txt',
+      '../secret.dart',


Can we add a test like this to dartSourceContents too?

expect(await webAssetServer.dartSourceContents('../secret.dart'), isNull); expect( await webAssetServer.dartSourceContents('packages/flutter_tools/../../secret.dart'), isNull, );

Added both to WebAssetServer does not serve files outside of the project. The first one fails without the lib/ change.

…h roots The web asset servers resolve the request path against each of their search roots with `Uri.resolve`. That treats the request path as a URI *reference*, not as a path: a reference carrying a scheme, an authority or an absolute path replaces the base instead of resolving below it, so the result is not necessarily inside the root that was being searched. Add two helpers to `web_server_utilities.dart`: * `isRelativeRequestPath` rejects a request path that is not a relative reference, i.e. one with a scheme or a Windows drive letter in its first segment, an authority, an absolute path, a backslash or a NUL. * `resolveRequestPathUnder` resolves against a given root and returns the result only when it lands inside that root, compared with `path.isWithin` so the comparison is per-component and case-insensitive on Windows. Every `resolve()` call in `ReleaseAssetServer.handle` and `WebAssetServer.handleRequest` now goes through the helper, so each search root keeps its own boundary: the canvaskit fallback, the built asset fallback, the project `web` directory, and the three roots consulted by `_resolveDartFile`. `.` and `..` segments are deliberately still accepted and handled by the containment check, so a request path that normalizes back inside the root keeps working. Dot segments are rejected before package resolution, where one that climbs past the package name makes `PackageConfig.resolve` throw. This also keeps an `UnsupportedError` from escaping the handler: `GET /..%2F..%2F..%2Ftmp/foo` currently throws `Illegal path character` out of `toFilePath` instead of being answered. The existing "does not serve files outside of the project" test is extended with the reference shapes that resolve outside a search root, and the debug handler gains the equivalent test, including `dartSourceContents`.

mdebbar

I need to find another reviewer to approve this so we can merge it.

r0g18 · 2026-10-23T16:58:03Z

Thanks for the review and approval! I appreciate it. Looking forward to the second review.

…

On Fri 23 Oct 2026 at 18:54, Mouad Debbar ***@***.***> wrote: ***@***.**** approved this pull request. I need to find another reviewer to approve this so we can merge it. — Reply to this email directly, view it on GitHub <#194064?email_source=notifications&email_token=BDB3F6ZBHRCAYWU7SMWWNZD5VOENNA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKNRTGE3DONRYGA22M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-5631676805>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BDB3F6ZTOSINURZ5PYHI3QT5VOENNAVCNFSNUABEKJSXA33TNF2G64TZHMZTCNZZGI4DENB3JFZXG5LFHM2TONZUGYZTINZQGKQXMAQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

harryterkelsen · 2026-10-23T18:40:48Z

+      if (segments.first == 'packages' && !hasDotSegment) {
        final Uri? filePath = _packages.resolve(
          Uri(scheme: 'package', pathSegments: segments.skip(1)),
        );


There are two edge cases with package resolution here that bypass containment or cause unhandled exceptions:

Path traversal / search root escape via double slash:
If a request comes in for packages/<package>//<absolute_path> (e.g. packages/flutter_tools//home/user/secret.dart or on Windows packages/flutter_tools//C:/Windows/win.ini), isRelativeRequestPath returns true, but segments.skip(1) contains an empty segment, producing package:flutter_tools//home/user/secret.dart. When _packages.resolve resolves that URI, package_config strips the package name and resolves /home/user/secret.dart against package.packageUriRoot. Because of the leading slash, Uri.resolve treats it as an absolute path reference and resolves to file:///home/user/secret.dart on root, completely bypassing containment.

Unhandled ArgumentError on short paths:
If path is packages/foo.dart, segments.skip(1) is ['foo.dart']. Uri(scheme: 'package', pathSegments: ['foo.dart']) constructs package:foo.dart without a trailing path segment, causing _packages.resolve to throw an unhandled ArgumentError ("Package URIs must start with the package name followed by a '/'"), crashing the request with an HTTP 500 instead of returning 404.

We can address both by validating the segment count, looking up the package directly, and resolving the remaining path with resolveRequestPathUnder:

if (segments.length >= 3 && segments.first == 'packages') { final Package? package = _packages[segments[1]]; if (package != null) { final String relativePackagePath = segments.skip(2).join('/'); final Uri? packageFileUri = resolveRequestPathUnder( package.packageUriRoot, relativePackagePath, fileSystem: fs, windows: platform.isWindows, ); if (packageFileUri != null) { final File packageFile = fs.file(packageFileUri); if (packageFile.existsSync()) { return packageFile; } } } }

This guarantees containment below the package's root and avoids calling _packages.resolve with arbitrary URI segments.

Thanks, good catches. I reproduced both with tests before changing anything, then applied your suggestion in 63978ed. It also drops the dot-segment check, since containment below the package root now handles those like the other roots. The new cases are in WebAssetServer does not serve files outside of the project, plus a positive case for a dot segment that stays inside the package.

harryterkelsen · 2026-10-23T18:40:49Z

    } else {
      for (final (Uri uri, Set<String> allowedExtensions) in _searchPaths()) {
-        final Uri potential = uri.resolve(requestPath);
+        final Uri? potential = resolveRequestPathUnder(


As a minor optimization, we could check allowedExtensions against _fileSystem.path.extension(requestPath) before calling resolveRequestPathUnder. Since _flutterRoot and _currentDirectory are restricted to source maps (.dart, .map), checking the extension upfront allows us to skip URI resolution and toFilePath conversions on every regular asset/html request.

Done in 63978ed, the extension is checked against the request path before resolving. It matches the old check on the resolved path: resolution keeps the last segment unless it is . or .., and those have no extension.

`_resolveDartFile` no longer builds a `package:` URI from the request segments. It looks the package up by name and resolves the rest of the path below that package's `packageUriRoot` with `resolveRequestPathUnder`, like the other search roots. An absolute path after the package name (`packages/<package>//<path>`) can no longer replace the package root, and a request with nothing after the package name (`packages/foo.dart`) is answered instead of throwing an `ArgumentError` out of `PackageConfig.resolve`. This replaces the dot-segment check: containment handles dot segments here the same way it does for the other roots. `ReleaseAssetServer` now checks a root's allowed extensions against the request path before resolving it, so regular asset requests skip resolution for the source-map-only roots.

kevmoo · 2026-10-24T17:20:26Z

Thanks for the work here! 🙏

…s search roots (#195201) Cherry-pick of #194064 to beta (`flutter-3.50-candidate.0`). ### Impact Description: Tightens how the web development server in `flutter_tools` maps request paths to files, so every lookup stays within the directory it is meant to serve from. It also stops some malformed request paths from throwing out of the request handler. This only affects development with `flutter run` on web, not deployed apps. ### Changelog Description: [#194064](#194064) When running a web app with `flutter run`, the development server resolves some request paths incorrectly. ### Workaround: None. ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: In `packages/flutter_tools`, run `test/general.shard/web/devfs_web_test.dart` and `test/general.shard/web/web_asset_server_test.dart`. All tests pass on this branch, and the new tests fail without the fix. Note: `web_asset_server.dart` needed a small manual merge because #194053 isn't on the 3.50 branch. The logic is the same as on master.

…h roots (flutter#194064) `flutter_tools`' web asset servers resolve the incoming request path against each of their search roots with `Uri.resolve`. That treats the request path as a URI *reference* rather than as a path, and a reference carrying a scheme, an authority or an absolute path replaces the base instead of resolving below it. The result of a lookup is therefore not guaranteed to be inside the root that was being searched, and nothing downstream checks that it is. This PR makes the containment explicit instead of relying on the shape of the request path. Two helpers are added to `web_server_utilities.dart`: * `isRelativeRequestPath` rejects a request path that is not a relative reference: one with a scheme or a Windows drive letter in its first segment, an authority, an absolute path, a backslash, or a NUL. * `resolveRequestPathUnder` resolves against a given root and returns the result only when it lands inside that root, compared with `path.isWithin` so the comparison is per-component and case-insensitive on Windows. Every `resolve()` call in `ReleaseAssetServer.handle` and `WebAssetServer.handleRequest` now goes through the helper, so each search root keeps its own boundary: the canvaskit fallback, the built asset fallback, the project `web` directory, and the roots consulted by `_resolveDartFile`. `.` and `..` segments are deliberately still accepted and handled by the containment check, so a request path that normalizes back inside the root keeps working. A `/packages/<package>/<path>` request is resolved the same way below that package's `packageUriRoot`, looked up by name, instead of through a `package:` URI built from the request segments. `ReleaseAssetServer` checks a root's allowed extensions against the request path before resolving it, so regular asset requests skip resolution for the source-map-only roots. This also keeps exceptions from escaping the handler. On `master` today, `GET /..%2F..%2F..%2Ftmp/foo` throws `UnsupportedError: Illegal path character` out of `toFilePath`, because the decoded segment contains a separator and so names no file path at all. Likewise `GET /packages/foo.dart` throws an `ArgumentError` out of `PackageConfig.resolve`. Both are now answered like any other path that does not resolve inside a root. ### Tests `ReleaseAssetServer does not serve files outside of the project` is extended with the reference shapes that resolve outside a search root, and `WebAssetServer does not serve files outside of the project` is added as the debug-handler equivalent, including `dartSourceContents` and `/packages/` requests with an absolute path or nothing after the package name. Both fail against unmodified `lib/` and pass with the change. Positive tests cover a package path with a dot segment that stays inside the package root, and nested build output paths, including an asset name containing a colon in a later segment, which stays valid in a relative reference and must keep working. `dart analyze` and `dart format` are clean on the five touched files. Rebased on current `master`; locally both web suites (`devfs_web_test.dart`, `web_asset_server_test.dart`) pass in full, 84 tests. The 4 failures I mentioned in an earlier version of this description came from running them without `FLUTTER_ROOT` set. ### Issues There is no public issue tracking this; it came out of maintainer feedback rather than a filed report. Happy to open one if the process needs the link. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

github-actions Bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Oct 9, 2026

gemini-code-assist Bot reviewed Oct 9, 2026

View reviewed changes

mdebbar self-requested a review October 23, 2026 02:57

mdebbar reviewed Oct 23, 2026

View reviewed changes

r0g18 force-pushed the hardening/web-asset-server-containment branch from 348e577 to b1149ce Compare October 23, 2026 16:42

mdebbar added the CICD Run CI/CD label Oct 23, 2026

mdebbar previously approved these changes Oct 23, 2026

View reviewed changes

harryterkelsen self-requested a review October 23, 2026 18:17

harryterkelsen requested changes Oct 23, 2026

View reviewed changes

r0g18 dismissed mdebbar’s stale review via 63978ed October 23, 2026 19:52

flutter-dashboard Bot removed the CICD Run CI/CD label Oct 23, 2026

harryterkelsen approved these changes Oct 23, 2026

View reviewed changes

mdebbar approved these changes Oct 23, 2026

View reviewed changes

mdebbar added autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD labels Oct 23, 2026

auto-submit Bot added this pull request to the merge queue Oct 23, 2026

Merged via the queue into flutter:master with commit 8f49173 Oct 23, 2026
31 checks passed

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 23, 2026

mdebbar mentioned this pull request Oct 23, 2026

[beta] [flutter_tools] Contain web asset server path resolution to its search roots #195201

Merged

2 of 5 tasks

engine-flutter-autoroll mentioned this pull request Oct 24, 2026

Roll Flutter from a3d71d82160a to 2541e508405f (141 revisions) flutter/packages#13413

Closed

This was referenced Oct 26, 2026

Roll Flutter from a3d71d82160a to 950bf4e8aa75 (146 revisions) flutter/packages#13441

Closed

Roll Flutter from a3d71d82160a to 4e78d0e8bee3 (148 revisions) flutter/packages#13442

Closed

chunhtai mentioned this pull request Oct 29, 2026

Flutter Notable Commits - 2026 #181433

Open

	//
	// An empty set relies on the request path staying below its root, which
	// `resolveRequestPathUnder` enforces for every root listed below.

		// A dot segment would resolve outside the package root, which package
		// resolution does not guard against on its own.

Conversation

r0g18 commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tests

Issues

Pre-launch Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

r0g18 commented Oct 23, 2026 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kevmoo commented Oct 24, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

r0g18 commented Oct 9, 2026 •

edited

Loading