[extension_discovery] Prevent config.yaml symlinks from escaping package boundary by jonasfj · Pull Request #2556 · dart-lang/tools

jonasfj · 2026-08-24T10:44:57Z

Disallow config.yaml from being a symlink pointing outside the package root.

b/536195690

During extension discovery, maliciously crafted symlinks for config.yaml could allow an attacker to read and cache sensitive host files outside the package directory. This commit enforces that the fully resolved canonical path of config.yaml resides within the upstream package's canonical boundary, mitigating the path escape.

github-actions · 2026-08-24T10:45:48Z

Package publishing

If you have publishing permissions, you can use the links below to publish the changes after merging this PR.

Package	Version	Status	Publish tag (post-merge)
package:api_summary	0.1.0-wip	WIP (no publish necessary)
package:bazel_worker	1.1.5	already published at pub.dev
package:benchmark_harness	2.4.0	already published at pub.dev
package:boolean_selector	2.1.2	already published at pub.dev
package:browser_launcher	1.2.0-wip	WIP (no publish necessary)
package:cli_config	0.2.1-wip	WIP (no publish necessary)
package:cli_util	0.6.1-wip	WIP (no publish necessary)
package:clock	1.1.3-wip	WIP (no publish necessary)
package:code_builder	4.12.0-wip	WIP (no publish necessary)
package:coverage	1.16.0-wip	WIP (no publish necessary)
package:csslib	1.0.3-wip	WIP (no publish necessary)
package:extension_discovery	2.1.1-wip	WIP (no publish necessary)
package:file	7.0.2-wip	WIP (no publish necessary)
package:file_testing	3.1.0-wip	WIP (no publish necessary)
package:glob	2.2.0-wip	WIP (no publish necessary)
package:graphs	2.4.0-wip	WIP (no publish necessary)
package:html	0.15.7-wip	WIP (no publish necessary)
package:io	1.1.0-wip	WIP (no publish necessary)
package:json_rpc_2	4.1.0	already published at pub.dev
package:markdown	7.4.1-wip	WIP (no publish necessary)
package:mime	2.1.0-wip	WIP (no publish necessary)
package:oauth2	2.0.5	already published at pub.dev
package:package_config	3.0.1-wip	WIP (no publish necessary)
package:pool	1.5.3-wip	WIP (no publish necessary)
package:process	5.0.6-wip	WIP (no publish necessary)
package:pub_semver	2.2.1-wip	WIP (no publish necessary)
package:pubspec_parse	1.6.0-wip	WIP (no publish necessary)
package:source_map_stack_trace	2.1.3-wip	WIP (no publish necessary)
package:source_maps	0.10.14	already published at pub.dev
package:source_span	1.10.2	already published at pub.dev
package:sse	4.2.1-wip	WIP (no publish necessary)
package:stack_trace	1.12.2-wip	WIP (no publish necessary)
package:stream_channel	2.1.4	already published at pub.dev
package:stream_transform	2.1.2-wip	WIP (no publish necessary)
package:string_scanner	1.4.2-wip	WIP (no publish necessary)
package:term_glyph	1.2.3-wip	WIP (no publish necessary)
package:test_reflective_loader	0.6.0	ready to publish	`test_reflective_loader-v0.6.0`
package:timing	1.0.2	already published at pub.dev
package:unified_analytics	8.0.18-wip	WIP (no publish necessary)
package:watcher	1.2.2-wip	WIP (no publish necessary)
package:yaml	3.1.4-wip	WIP (no publish necessary)
package:yaml_edit	2.2.5-wip	WIP (no publish necessary)

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

gemini-code-assist

Code Review

This pull request introduces a security fix to prevent symlinks from causing config.yaml to be read from outside the package folder, mitigating potential symlink cache poisoning. It implements a boundary check using resolveSymbolicLinksSync() and adds corresponding tests. A review comment highlights that _isWithinPackageBoundary is called outside the try-catch block, which could cause findExtensions to crash if resolveSymbolicLinksSync() throws an IOException. It is recommended to move this check inside the try block to safely handle any exceptions.

github-actions · 2026-08-24T10:46:41Z

PR Health

Unused Dependencies

⚠️

Package	Status
extension_discovery	❗ Show Issues These packages are used outside lib/ but are not dev_dependencies: * hello_world

For details on how to fix these, see dependency_validator.

This check can be disabled by tagging the PR with skip-unused-dependencies-check.

Breaking changes ✔️

Package	Change	Current Version	New Version	Needed Version	Looking good?
extension_discovery	Non-Breaking	2.1.0	2.1.1-wip	2.1.1-wip	✔️

This check can be disabled by tagging the PR with skip-breaking-check.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package	Leaked API symbol	Leaking sources

This check can be disabled by tagging the PR with skip-leaking-check.

Changelog Entry ✔️

Package	Changed Files

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

License Headers ✔️

// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers

Files
pkgs/html/test/differential_test.dart
pkgs/html/test/dom_parsing_test.dart
pkgs/html/test/mxss_test.dart
pkgs/html/test/noah_ark_clause_limit_test.dart

This check can be disabled by tagging the PR with skip-license-check.

Coverage ✔️

File	Coverage
pkgs/extension_discovery/lib/extension_discovery.dart	💚 59 % ⬆️ Infinity %

This check for test coverage is informational (issues shown here will not fail the PR).

This check can be disabled by tagging the PR with skip-coverage-check.

natebosch · 2026-08-25T23:12:37Z

-            packageUri: p.packageUri,
-            config: parseYamlFromConfigFile(await configFile.readAsString()),
-          );
+          if (_isWithinPackageBoundary(rootUri, configFile)) {


How about

if (configStat.isFileOrLink && _isWithinPackageBoundary(rootUri, configFile)) {

that would have been a bit more elegant :D

for the record it just landed via auto-merge while I was sleeping.

natebosch · 2026-08-25T23:14:29Z

        if (configStat.isPossiblyModifiedAfter(registryStat.modified)) {
-          try {
-            registryUpdated = true;
-            registry[i] = (
-              package: p.package,
-              rootUri: p.rootUri,
-              packageUri: p.packageUri,
-              config: parseYamlFromConfigFile(await configFile.readAsString()),
-            );
-            continue;
-          } on FormatException {
-            // pass
-          } on IOException {
-            // pass
+          if (_isWithinPackageBoundary(rootUri, configFile)) {


How about

if (configStat.isPossiblyModifiedAfter(registryStat.modified) && _isWithinPackageBoundary(rootUri, configFile)) {

Filed #2557

natebosch · 2026-08-25T23:17:54Z

 Uri fileUri(String path) => Uri.file(d.path(path));
 Uri directoryUri(String path) => Uri.directory(d.path(path));

+class _LinkDescriptor extends d.Descriptor {


Should we put this in package:test_descriptor? I imagine we'll need this in other places.

If we want to expand package:test_descriptor we totally could, there is also a bunch of extensions to it in dart-lang/pub.

I don't want to block this PR on it.

And I fear that if I spend much time on test_descriptor I'll want to do bigger changes 🙈 🙈 🙈

jonasfj requested review from a team as code owners August 24, 2026 10:44

github-actions Bot added the package:extension_discovery label Aug 24, 2026

jonasfj requested review from kenzieschmoll and sigurdm August 24, 2026 10:45

jonasfj enabled auto-merge (squash) August 24, 2026 10:45

gemini-code-assist Bot reviewed Aug 24, 2026

View reviewed changes

Comment thread pkgs/extension_discovery/lib/extension_discovery.dart Resolved

jonasfj added 3 commits August 24, 2026 10:58

Fix test formatting and analyzer warnings

eb68281

Fix analyzer warnings and unhandled IOExceptions

069b273

Add license header to new test file

4718cfc

natebosch approved these changes Aug 25, 2026

View reviewed changes

jonasfj merged commit 8108a7c into main Aug 25, 2026
21 checks passed

jonasfj deleted the fix_symlink_cache_poisoning branch August 25, 2026 23:18

jonasfj mentioned this pull request Aug 26, 2026

[extension_discovery] Fixup review comments #2557

Merged

Navigation Menu

Repository navigation

[extension_discovery] Prevent config.yaml symlinks from escaping package boundary - #2556

[extension_discovery] Prevent config.yaml symlinks from escaping package boundary#2556
jonasfj merged 4 commits into
mainfrom
fix_symlink_cache_poisoning

jonasfj commented Aug 24, 2026

Uh oh!

github-actions Bot commented Aug 24, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 24, 2026 •

edited

Loading

Uh oh!

natebosch Aug 25, 2026

Uh oh!

jonasfj Aug 26, 2026

Uh oh!

natebosch Aug 25, 2026

Uh oh!

jonasfj Aug 26, 2026

Uh oh!

natebosch Aug 25, 2026

Uh oh!

jonasfj Aug 26, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jonasfj commented Aug 24, 2026

Uh oh!

github-actions Bot commented Aug 24, 2026

Package publishing

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

github-actions Bot commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Health

Uh oh!

natebosch Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

jonasfj Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

natebosch Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

jonasfj Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

natebosch Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

jonasfj Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Aug 24, 2026 •

edited

Loading