Skip to content

Follow symlinks in precompiled browser test directories - #2787

Merged
natebosch merged 1 commit into
masterfrom
precompiled-follow-symlinks
Oct 12, 2026
Merged

Follow symlinks in precompiled browser test directories#2787
natebosch merged 1 commit into
masterfrom
precompiled-follow-symlinks

Conversation

@natebosch

@natebosch natebosch commented Oct 9, 2026

Copy link
Copy Markdown
Member

Fixes #2766

#2724 removed serveFilesOutsidePath: true from the precompiled static
handler to stop requests from reading files outside the precompiled
directory. Without that flag shelf_static also refuses to serve any
symlink whose target resolves outside the root. build_runner test
defaults to --symlink on non-Windows platforms. That creates a merged
output directory where every file is a symlink into .dart_tool/build or
the package sources, so every request for a compiled test returned 404 and
tests failed to load with "Failed to load script at ...".

Check containment on the requested path instead of the symlink-resolved
path. The new createSymlinkFollowingStaticHandler joins the request path
segments onto the root the same way shelf_static does, rejects the
request if the normalized result is outside the root, and otherwise
delegates to a static handler with serveFilesOutsidePath: true. Symlinks
placed inside the precompiled directory are followed. Traversal through
the request path, such as ..%2F segments or an encoded absolute path,
is still rejected.

Add unit tests that send raw HTTP requests to cover symlinked files and
directories and several traversal forms. With the containment check
disabled, the encoded-slash and absolute-path cases serve the outside
file. Add a Chrome integration test that runs a precompiled test from a
directory of symlinks; it fails with the previous handler.

@natebosch
Follow symlinks in precompiled browser test directories
ee29ba5 
Fixes #2766

#2724 removed `serveFilesOutsidePath: true` from the precompiled static
handler to stop requests from reading files outside the precompiled
directory. Without that flag `shelf_static` also refuses to serve any
symlink whose target resolves outside the root. `build_runner test`
defaults to `--symlink` on non-Windows platforms. That creates a merged
output directory where every file is a symlink into `.dart_tool/build` or
the package sources, so every request for a compiled test returned 404 and
tests failed to load with "Failed to load script at ...".

Check containment on the requested path instead of the symlink-resolved
path. The new `createSymlinkFollowingStaticHandler` joins the request path
segments onto the root the same way `shelf_static` does, rejects the
request if the normalized result is outside the root, and otherwise
delegates to a static handler with `serveFilesOutsidePath: true`. Symlinks
placed inside the precompiled directory are followed. Traversal through
the request path, such as `..%2F` segments or an encoded absolute path,
is still rejected.

Add unit tests that send raw HTTP requests to cover symlinked files and
directories and several traversal forms. With the containment check
disabled, the encoded-slash and absolute-path cases serve the outside
file. Add a Chrome integration test that runs a precompiled test from a
directory of symlinks; it fails with the previous handler.
@natebosch
natebosch requested a review from a team as a code owner October 9, 2026 02:37
@natebosch
natebosch requested a review from jakemac53 October 9, 2026 02:37
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

PR Health

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.

jakemac53
jakemac53 approved these changes Oct 12, 2026
View reviewed changes
Comment thread pkgs/test/lib/src/util/symlink_following_static_handler.dart
final fsPath = p.normalize(
p.joinAll([resolvedRoot, ...request.url.pathSegments]),
);
if (!p.isWithin(resolvedRoot, fsPath)) {

@jakemac53 jakemac53 Oct 12, 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.

Will this block requests to /? Do we need that to work?

@natebosch natebosch Oct 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, it will block requests to /.

No, we don't need it to work.

Our current usage of createStaticHandler passed neither the defaultDocument or listDirectories arguments so request to / will 404 both before and after this change.

@natebosch
natebosch merged commit 17c9849 into master Oct 12, 2026
71 checks passed
@natebosch
natebosch deleted the precompiled-follow-symlinks branch October 12, 2026 19:08
copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request Oct 13, 2026
@natebosch dart-scoped@luci-project-accounts.iam.gserviceaccount.com
[deps] rev dartdoc, ecosystem, http, native, test, tools
070ca6e 
Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/5eb0f2c..e1859cf):
  e1859cfc  Fri Oct 9 14:53:01 2026 +0200  Sarah Zakarias  Link a bare prefix only when it maps to exactly one library (dart-lang/dartdoc#4295)
  c48f6fb9  Fri Oct 9 14:25:42 2026 +0200  Sarah Zakarias  Resolve prefixes through the analyzer's PrefixElement.scopeLibraries (dart-lang/dartdoc#4294)
  3a6297e2  Wed Oct 7 12:56:14 2026 +0200  Sarah Zakarias  Require analyzer 14.5.0 and bump to 9.0.10 (dart-lang/dartdoc#4292)

ecosystem (https://github.com/dart-lang/ecosystem/compare/16dca7a..622ef61):
  622ef61  Mon Oct 12 00:23:27 2026 -0700  Kevin Moore  fix(firehose): enforce -wip on published versions and tighten changelog checks (dart-lang/ecosystem#467)
  5576a85  Wed Oct 7 09:29:44 2026 +0200  Moritz  Support use-flutter option in groundskeeper workflow (dart-lang/ecosystem#444)

http (https://github.com/dart-lang/http/compare/585d433..23f8574):
  23f8574  Thu Oct 8 23:27:25 2026 +0700  Cong Bang  feat(http2): configurable receive windows and frame size (dart-lang/http#1999)
  18051a6  Thu Oct 8 10:39:38 2026 +0200  Moritz  Enable Flutter in Groundskeeper workflow for Flutter packages (dart-lang/http#2004)

native (https://github.com/dart-lang/native/compare/a2a6179..10515af):
  10515afca  Mon Oct 12 12:21:52 2026 +0200  Daco Harkes  [native_toolchain_c] Do not pass linker flags when compiling static libraries (dart-lang/native#3731)

native is held back at 10515af, before dart-lang/native#3728, because it
breaks the dartdev `dart install` native assets tests. See
dart-lang/native#3743.

test (https://github.com/dart-lang/test/compare/d749b10..30acb69):
  30acb69e  Mon Oct 12 12:42:54 2026 -0700  Nate Bosch  Time out browser loads just before the suite load timeout (dart-lang/test#2786)
  597a0326  Mon Oct 12 12:22:53 2026 -0700  Kevin Moore  Fix misplaced changelog entries and remove no-op health check (dart-lang/test#2789)
  0b6efee8  Mon Oct 12 12:16:29 2026 -0700  Nate Bosch  [checks] Collapse nested subjects in condition descriptions (dart-lang/test#2784)
  17c98493  Mon Oct 12 12:08:23 2026 -0700  Nate Bosch  Follow symlinks in precompiled browser test directories (dart-lang/test#2787)
  1cc813ec  Fri Oct 9 11:59:56 2026 -0700  Nate Bosch  [checks] Join nested collapsed subjects with "that" (dart-lang/test#2783)
  ada7b950  Thu Oct 8 12:03:42 2026 -0700  Nate Bosch  Prepare to publish (dart-lang/test#2780)
  c4b165b3  Wed Oct 7 11:29:08 2026 -0700  Kevin Moore  [checks] Require package:meta ^1.17.0 (dart-lang/test#2779)
  4a363d70  Tue Oct 6 10:46:31 2026 -0700  Nate Bosch  Let Chrome choose its remote debugging port (dart-lang/test#2778)
  465d66ab  Tue Oct 6 09:35:22 2026 -0700  Nate Bosch  Keep Chrome stderr in the browser output when debugging (dart-lang/test#2777)

tools (https://github.com/dart-lang/tools/compare/a66d517..2b54c71):
  2b54c71af  Mon Oct 12 10:16:45 2026 +0200  dart-ecosystem-bot[bot]  [boring] Update BoringSSL to 3fed4f20 (dart-lang/tools#2655)
  2337f995a  Sat Oct 10 03:11:40 2026 +0300  Hasan Armoush  [json_rpc_2] Add strictProtocolChecks parameter to Client (dart-lang/tools#2555)
  c65c69ed1  Fri Oct 9 08:54:25 2026 +0200  Moritz  [boring] Move package:boring to dart-lang/tools (2/4): FFI helpers from package:webcrypto (dart-lang/tools#2615)
  a04e45491  Thu Oct 8 21:07:18 2026 -0700  Kevin Moore  chore: wire package:api_summary and package:string_scanner PR auto-labeler (dart-lang/tools#2652)
  8bdd4b92c  Thu Oct 8 09:20:48 2026 +0200  Morgan :)  [code_builder] Allow built_collection 6.x. (dart-lang/tools#2645)
  c16040d35  Wed Oct 7 17:30:08 2026 -0700  Kevin Moore  [api_summary] Sort primary barrel first and experimental entry points last (v1.2.0) (dart-lang/tools#2619)
  b2d0780db  Wed Oct 7 11:27:16 2026 -0700  Kevin Moore  [stream_channel] Require package:async ^2.6.0 (dart-lang/tools#2644)
  3ab7bf9d0  Wed Oct 7 11:35:27 2026 +0200  Moritz  [boring] Move package:boring to dart-lang/tools (1/4): vendored BoringSSL, bindings and source build (dart-lang/tools#2614)
  29df6348b  Tue Oct 6 15:21:44 2026 -0700  Kevin Moore  feat(coverage): support web coverage reports in test_with_coverage (dart-lang/tools#2448)

Regenerate pkg/{analyzer,analyzer_plugin,analyzer_testing}/api.txt. The
rolled package:api_summary v1.2.0 (dart-lang/tools#2619) sorts the primary
barrel first and other entry points after it, so entries in these files
are reordered. The APIs themselves are unchanged.

Change-Id: Ie8ff19c5a981ea8ecee8939c75457986a947e0a7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/563220
Reviewed-by: Samuel Rawlins <[email protected]>
Auto-Submit: Nate Bosch <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jakemac53 jakemac53 jakemac53 approved these changes

Assignees

No one assigned

Labels

package:test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

build_runner-precompiled tests fail to load in test 1.32.0

2 participants

@natebosch @jakemac53