Skip to content

[flutter_tools] Enforce that package-declared asset paths stay within the package#187661

Merged
auto-submit[bot] merged 3 commits into
flutter:masterfrom
adilburaksen:asset-package-dir-containment
Jun 17, 2026
Merged

[flutter_tools] Enforce that package-declared asset paths stay within the package#187661
auto-submit[bot] merged 3 commits into
flutter:masterfrom
adilburaksen:asset-package-dir-containment

Conversation

@adilburaksen

@adilburaksen adilburaksen commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Fixes a containment gap in asset resolution.

Problem

_ensureAssetPathIsValid (packages/flutter_tools/lib/src/asset.dart) rejects absolute asset paths but allows a relative path containing .. to escape the base directory, and _Asset.lookupAssetFile joins the base dir with the (uncontained) relative path. For a dependency, the asset base is that package’s own directory, so a package can declare an asset such as ../something and cause a consumer’s flutter build to read a file from outside the package on the build machine and bundle it into the app — even though the consuming app never declared that path.

Fix

Add a containment check for dependency-declared assets: the resolved path must stay within the package directory, mirroring the existing absolute-path rejection. The check is scoped to packageName != null, so app-level asset paths are unaffected and existing first-party .. asset usage keeps working.

Tests

  • New unit test in asset_bundle_package_test.dart: a dependency asset that escapes its package directory is rejected.
  • Verified with a real flutter build bundle: an escaping dependency asset is now rejected, while a normal in-package dependency asset still bundles correctly.
  • All existing asset_bundle_package_test cases pass.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making.
  • I followed the breaking change policy and added Data Driven Fixes where supported.
  • All existing and new tests are passing.

@adilburaksen
[flutter_tools] Enforce that package-declared asset paths stay within…
51f6da5 
… the package

`_ensureAssetPathIsValid` rejected absolute asset paths but allowed a relative
path containing `..` to escape the base directory, and `_Asset.lookupAssetFile`
joins the base dir with the (uncontained) relative path. For a dependency, the
asset base is that package's own directory, so a package could declare an asset
such as `../secret` and cause a consumer's `flutter build` to read a file from
outside the package on the build machine and bundle it into the app — which the
consuming app never declared.

This adds a containment check for dependency-declared assets: the resolved path
must stay within the package directory (mirroring the existing absolute-path
rejection). App-level asset paths are unaffected (the check is scoped to
`packageName != null`), so existing first-party `..` asset usage keeps working.

Verified with a real `flutter build bundle` (escaping dependency asset now
rejected; a normal in-package dependency asset still bundles) and a new unit
test; all existing asset_bundle_package_test cases pass.
@github-actions github-actions Bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 7, 2026
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Jun 7, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces validation to ensure that asset paths declared by package dependencies do not escape their package directory, throwing a tool exit if they resolve outside of it. A corresponding test case has been added to verify this behavior. Feedback suggests extracting the repeated evaluation of the asset file path into a local variable to improve readability and avoid redundant operations.

Comment thread packages/flutter_tools/lib/src/asset.dart
Comment on lines +1359 to +1371
if (packageName != null) {
final String base = _fileSystem.path.canonicalize(assetsBaseDir);
final String resolved = _fileSystem.path.canonicalize(
_fileSystem.path.join(assetsBaseDir, assetUri.toFilePath(windows: _platform.isWindows)),
);
if (base != resolved && !_fileSystem.path.isWithin(base, resolved)) {
throwToolExit(
'Asset path "${assetUri.toFilePath(windows: _platform.isWindows)}" of package '
'"$packageName" resolves to a location outside the package directory. Package asset '
'paths must stay within the package.',
);
}
}

@gemini-code-assist gemini-code-assist Bot Jun 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The expression assetUri.toFilePath(windows: _platform.isWindows) is evaluated twice within this block. Extracting it into a local variable improves readability and avoids redundant operations.

    if (packageName != null) {
      final String assetPath = assetUri.toFilePath(windows: _platform.isWindows);
      final String base = _fileSystem.path.canonicalize(assetsBaseDir);
      final String resolved = _fileSystem.path.canonicalize(
        _fileSystem.path.join(assetsBaseDir, assetPath),
      );
      if (base != resolved && !_fileSystem.path.isWithin(base, resolved)) {
        throwToolExit(
          'Asset path "$assetPath" of package '
          '"$packageName" resolves to a location outside the package directory. Package asset '
          'paths must stay within the package.',
        );
      }
    }

@bkonyi bkonyi Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@adilburaksen adilburaksen Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — extracted assetPath, thanks.

@bkonyi bkonyi requested a review from chingjun June 16, 2026 21:15
@bkonyi bkonyi added team-tool Owned by Flutter Tool team CICD Run CI/CD labels Jun 16, 2026
bkonyi
bkonyi reviewed Jun 16, 2026
View reviewed changes

@bkonyi bkonyi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me overall! Just one minor comment.

@github-actions github-actions Bot removed team-tool Owned by Flutter Tool team CICD Run CI/CD labels Jun 16, 2026
@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 17, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 17, 2026
@auto-submit

auto-submit Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/187661, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR.

@bkonyi bkonyi added autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD labels Jun 17, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 17, 2026
@auto-submit

auto-submit Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/187661, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR.

@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 17, 2026
@bkonyi bkonyi added autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD labels Jun 17, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue Jun 17, 2026
Merged via the queue into flutter:master with commit 53dfad2 Jun 17, 2026
165 of 166 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 17, 2026
This was referenced Jun 18, 2026
Closed
Closed
Closed
This was referenced Jun 18, 2026
Closed
Closed
Merged
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Jun 18, 2026
@engine-flutter-autoroll
Roll Flutter from b10d0f1ac615 to 15963bc04c2b (30 revisions) (#11940)
c516c92 
flutter/flutter@b10d0f1...15963bc

2026-06-18 engine-flutter-autoroll@skia.org Roll Skia from 6e84902d56c3 to 1ae2466c9ea5 (4 revisions) (flutter/flutter#188172)
2026-06-18 engine-flutter-autoroll@skia.org Roll Packages from 6ce00a8 to 4fd05e6 (3 revisions) (flutter/flutter#188171)
2026-06-18 robert.ancell@canonical.com [Linux] Fix vertical offset in composite_layer (flutter/flutter#188145)
2026-06-18 robert.ancell@canonical.com [Linux] Fix incorrect GL datatypes for uniform locations (flutter/flutter#188143)
2026-06-18 engine-flutter-autoroll@skia.org Roll Dart SDK from e05c69222ea4 to 5883736e7670 (2 revisions) (flutter/flutter#188168)
2026-06-18 engine-flutter-autoroll@skia.org Roll Skia from 046277850e8d to 6e84902d56c3 (5 revisions) (flutter/flutter#188165)
2026-06-18 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from or21OEdGtairm6nl9... to 1E2qOlNnC2Ucn-1oV... (flutter/flutter#188162)
2026-06-18 engine-flutter-autoroll@skia.org Roll Skia from 8dd207d443d3 to 046277850e8d (1 revision) (flutter/flutter#188153)
2026-06-18 31859944+LongCatIsLooong@users.noreply.github.com Add entitlements.txt entries for new dart sdk binaries (flutter/flutter#188133)
2026-06-18 engine-flutter-autoroll@skia.org Roll Dart SDK from b670723c5f07 to e05c69222ea4 (1 revision) (flutter/flutter#188146)
2026-06-18 robert.ancell@canonical.com Fix bounds checking in FlAccessibleTextField (flutter/flutter#188137)
2026-06-18 engine-flutter-autoroll@skia.org Roll Skia from f5a2921fe23e to 8dd207d443d3 (2 revisions) (flutter/flutter#188141)
2026-06-18 30870216+gaaclarke@users.noreply.github.com Adds tests for disabling macos impeller (flutter/flutter#188132)
2026-06-17 jlemanski1@gmail.com Improve Flutter Web accessibility: update flt meta viewport tag to align with WCAG 2 guidelines (flutter/flutter#182047)
2026-06-17 engine-flutter-autoroll@skia.org Roll Dart SDK from e39bde5b1bfc to b670723c5f07 (2 revisions) (flutter/flutter#188130)
2026-06-17 engine-flutter-autoroll@skia.org Roll Skia from 066bfbac7282 to f5a2921fe23e (1 revision) (flutter/flutter#188128)
2026-06-17 matt.boetger@gmail.com Support --trace-systrace in release builds on Android (flutter/flutter#186359)
2026-06-17 matt.boetger@gmail.com Isolate compiled dill caches by TargetModel (flutter/flutter#187253)
2026-06-17 98614782+auto-submit[bot]@users.noreply.github.com Reverts "refactor(web): Unify Image on Skwasm and CanvasKit (#187873)" (flutter/flutter#188124)
2026-06-17 matt.kosarek@canonical.com Use a mock EGL manager in windows unittests to avoid flaky rendering calls (flutter/flutter#188078)
2026-06-17 matt.boetger@gmail.com [Android] Remove support for unused manifest flags (flutter/flutter#186021)
2026-06-17 30870216+gaaclarke@users.noreply.github.com Adds windows project switch for enabling impeller (flutter/flutter#188044)
2026-06-17 15619084+vashworth@users.noreply.github.com Skip prefetch SwiftPM dependencies if the project hasn't been migrated to SwiftPM yet (flutter/flutter#187206)
2026-06-17 nshahan@google.com [flutter_tools] Bump dwds to 27.1.2 (flutter/flutter#187951)
2026-06-17 30870216+gaaclarke@users.noreply.github.com Adds external texture devicelab test for windows impeller (flutter/flutter#187886)
2026-06-17 engine-flutter-autoroll@skia.org Roll Skia from 5d19002eb73e to 066bfbac7282 (2 revisions) (flutter/flutter#188118)
2026-06-17 34871572+gmackall@users.noreply.github.com Add note about magnifier issue when using transparent HCPP pv (flutter/flutter#187753)
2026-06-17 30870216+gaaclarke@users.noreply.github.com [linux]: fixes crash when resizing windows (flutter/flutter#187626)
2026-06-17 56400880+adilburaksen@users.noreply.github.com [flutter_tools] Enforce that package-declared asset paths stay within the package (flutter/flutter#187661)
2026-06-17 jason-simmons@users.noreply.github.com Remove canvaskit_cipd_instance from DEPS (flutter/flutter#188073)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC bmparr@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jun 18, 2026
@adilburaksen @bkonyi @LouiseHsu
[flutter_tools] Enforce that package-declared asset paths stay within…
8be7316 
… the package (flutter#187661)

Fixes a containment gap in asset resolution.

## Problem
`_ensureAssetPathIsValid` (`packages/flutter_tools/lib/src/asset.dart`)
rejects absolute asset paths but allows a relative path containing `..`
to escape the base directory, and `_Asset.lookupAssetFile` joins the
base dir with the (uncontained) relative path. For a **dependency**, the
asset base is that package’s own directory, so a package can declare an
asset such as `../something` and cause a consumer’s `flutter build` to
read a file from **outside the package** on the build machine and bundle
it into the app — even though the consuming app never declared that
path.

## Fix
Add a containment check for dependency-declared assets: the resolved
path must stay within the package directory, mirroring the existing
absolute-path rejection. The check is scoped to `packageName != null`,
so **app-level asset paths are unaffected** and existing first-party
`..` asset usage keeps working.

## Tests
- New unit test in `asset_bundle_package_test.dart`: a dependency asset
that escapes its package directory is rejected.
- Verified with a real `flutter build bundle`: an escaping dependency
asset is now rejected, while a normal in-package dependency asset still
bundles correctly.
- All existing `asset_bundle_package_test` cases pass.

## Pre-launch Checklist
- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making.
- [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/master/docs/contributing/Tree-hygiene.md#overview
[breaking change policy]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Data-driven-Fixes.md

---------

Co-authored-by: Ben Konyi <bkonyi@google.com>
via-guy pushed a commit to via-guy/flutter that referenced this pull request Jun 26, 2026
@adilburaksen @bkonyi @via-guy
[flutter_tools] Enforce that package-declared asset paths stay within…
72feca9 
… the package (flutter#187661)

Fixes a containment gap in asset resolution.

## Problem
`_ensureAssetPathIsValid` (`packages/flutter_tools/lib/src/asset.dart`)
rejects absolute asset paths but allows a relative path containing `..`
to escape the base directory, and `_Asset.lookupAssetFile` joins the
base dir with the (uncontained) relative path. For a **dependency**, the
asset base is that package’s own directory, so a package can declare an
asset such as `../something` and cause a consumer’s `flutter build` to
read a file from **outside the package** on the build machine and bundle
it into the app — even though the consuming app never declared that
path.

## Fix
Add a containment check for dependency-declared assets: the resolved
path must stay within the package directory, mirroring the existing
absolute-path rejection. The check is scoped to `packageName != null`,
so **app-level asset paths are unaffected** and existing first-party
`..` asset usage keeps working.

## Tests
- New unit test in `asset_bundle_package_test.dart`: a dependency asset
that escapes its package directory is rejected.
- Verified with a real `flutter build bundle`: an escaping dependency
asset is now rejected, while a normal in-package dependency asset still
bundles correctly.
- All existing `asset_bundle_package_test` cases pass.

## Pre-launch Checklist
- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making.
- [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/master/docs/contributing/Tree-hygiene.md#overview
[breaking change policy]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Data-driven-Fixes.md

---------

Co-authored-by: Ben Konyi <bkonyi@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chingjun chingjun chingjun approved these changes
@bkonyi bkonyi bkonyi approved these changes
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CICD Run CI/CD tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@adilburaksen @chingjun @bkonyi