Use -- to separate chrome URLs

[natebosch]

If any URLs are passed with leading -- characters they may be
interpreted as arguments instead. Any arguments which follow a bare --
are explicitly treated as URLs not arguments. Avoid the need for callers
to sanitize arguments with this explicit divider.

Drop an unnecessary private implementation since it's small enough for a
direct public implementation that's as small as the forwarder anyway.

BUG=b/536185918

[gemini-code-assist]

Code Review

This pull request refactors the Chrome process launching logic by removing the private _startProcess helper and simplifying the start method to directly call Process.start using spread operators. Feedback suggests conditionally adding the -- argument separator only when urls is not empty to avoid potential issues with Chrome when no URLs are provided.

[github-actions]

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.5.2	already published at pub.dev
package:clock	1.1.3-wip	WIP (no publish necessary)
package:code_builder	4.12.0-wip	WIP (no publish necessary)
package:coverage	1.15.1	already published at pub.dev
package:csslib	1.0.2	already published at pub.dev
package:extension_discovery	2.1.0	already published at pub.dev
package:file	7.0.2-wip	WIP (no publish necessary)
package:file_testing	3.1.0-wip	WIP (no publish necessary)
package:glob	2.1.4-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.0	ready to publish	markdown-v7.4.0
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.0	already published at pub.dev
package:pool	1.5.3-wip	WIP (no publish necessary)
package:process	5.0.5	(error) pubspec version (5.0.5) and changelog (5.0.6-wip) don't agree
package:pub_semver	2.2.0	already published at pub.dev
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-wip	WIP (no publish necessary)
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	(error) pubspec version (1.12.2-wip) and changelog (1.12.2-dev) don't agree
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.16	already published at pub.dev
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.4	already published at pub.dev

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

[github-actions]

PR Health

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.

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

Unused Dependencies ✔️

Package	Status
browser_launcher	✔️ All dependencies utilized correctly.

For details on how to fix these, see dependency_validator.

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

Coverage ⚠️

File	Coverage
pkgs/browser_launcher/lib/src/chrome.dart	💔 69 % ⬇️ 1 %

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.

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.

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.

Breaking changes ✔️

Package	Change	Current Version	New Version	Needed Version	Looking good?
browser_launcher	Non-Breaking	1.1.3	1.2.0-wip	1.2.0-wip	✔️

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

[lrhn]

(Can be moved into literal, even if it needs to be last.)

[natebosch]

Done.

[lrhn]

Who calls this function?
Can we trust additionalArguments to not already contain -- and/or a non-argument?

If it's called as startWithDebugPort([url2Text], additionalArguments: ['--', url1Text]),
it looks like it would currently work , so if it's a public function, someone might be doing that.
(A -- would break if headless is true, since it'd add --headless after the --, and then fail to treat it as a URL, but if headless is false, it could work.)

Adding an extra -- will likely give an error after trying to use the second -- as a URL, making this a breaking change, even if for a very questionable use.

Should we validate that additionalArguments are additional flags? Maybe that's non-trivial if you can have --flag value, so we'd have to understand the flag syntax to know if value belongs to a flag or will be read as a URL. (Can chrome switches be in two arguments like that? It seems much safer if they can't.)

Maybe just check that additionalArguments does not contain --.
I'd make it an error if it does, and then unconditionally add the -- before the URIs.

We also don't check if the URIs contain flags?
Which would also currently work.
So if someone passes urls as [--headless', myUrl], that would also currently work.
Definitely not intended use, but how confident are we about breaking it?

That is, we're preventing unvalidate URIs starting with -- from being treated as flags unintendedly. What if it was intended?

(I say "break the .....s, they had it coming", but that may just be me.)

So, my recommendation: Check if additionalArguments contain --, and throw an ArgumentError if it does. Then continue doing what this PR does.

[natebosch]

Who calls this function?
Can we trust additionalArguments to not already contain -- and/or a non-argument?

Some of our dev tooling (I think to localhost URLs we control) and potentially any third party use of browser launching.

For the use cases we support, I suspect we can trust it to not already contain -- or a non-argument, and it would be valid for us to just have that expectation.

Maybe just check that additionalArguments does not contain --.
I'd make it an error if it does, and then unconditionally add the -- before the URI

Yeah I'm leaning towards using an assert that args does not contain -- and unconditionally adding it.
It's even safe to include it when the URLs is an empty list so I think I'll do that.

So if someone passes urls as [--headless', myUrl], that would also currently work.
Definitely not intended use, but how confident are we about breaking it?

I think it is OK to break this (mis)use of the API. We are very unlikely to have used it this way in any of our supported use cases.

So, my recommendation: Check if additionalArguments contain --, and throw an ArgumentError if it does. Then continue doing what this PR does.

WDYT about using an assert?

[lrhn]

(See above. If not rejecting earlier if args already contains --, check here and only add -- if not.)

[natebosch]

Added assert

[lrhn]

What is the threat/misuse model here?

Is someone calling start or startWithDebugPort with user supplied URL strings that are not validated,
but the extra arguments are trusted?
Can the URL strings starting with -- ever be a valid and useful URL? If not, should we just reject early, throwing an exception, instead of putting a -- in front and making it a later "Bad URL" error?
(The URL arguments to chrome will not have a base-URI to resolve against. A scheme cannot start with -. A schemeless host must start with //. That makes --... a path only URI reference with not base URI. That's never useful.)

That is: Should we validate inputs more, and reject any extra-argument that does not start with - or -- or that is just --,
and/or reject any URL that starts with -?

(Just putting -- in front of the URLs feeels like it's only protecting against a specific use-case, instead of the general problem.)

[natebosch]

What is the threat/misuse model here?

See b/536185918

The theory is that a malicious actor is able to control some URL that will be launched by a dev tool, they use this exploit to pass a flag like --renderer-cmd-prefix and then somehow graduate that to an RCE.

Is someone calling start or startWithDebugPort with user supplied URL strings that are not validated,
but the extra arguments are trusted?

Not that I can think of, but maybe? I think our use is with localhost URLs that aren't under the control of third party packages, but I also don't want to track down every use of this API and verify if they're using it in a safe way.

We don't explicitly support usage outside our tools, but it's always possible some third party is using this in an unsafe way and would be helped by this fix. It shouldn't hurt to protect from poorly validated use even if we're using it safely today.

Can the URL strings starting with -- ever be a valid and useful URL? If not, should we just reject early, throwing an exception, instead of putting a -- in front and making it a later "Bad URL" error?

I'm not sure, probably not, but I think I'm happy for chrome to own that knowledge instead of us. Passing a "bad" URL that starts with a -- will open up an error page in chrome, which I think is a fine result for that case.

That is: Should we validate inputs more, and reject any extra-argument that does not start with - or -- or that is just --,
and/or reject any URL that starts with -?

IMO no. It's quite plausible that there is some developer friendliness in an approach like that (fail early and more clearly), but the usage is so narrow it's worth taking a shortcut and letting chrome own this stuff. I think we do need to protect against callers who passed their own -- at the end of args though.

Callers should be either controlling or validating the args they pass, if they aren't doing that nothing we can do here will help them. It's reasonable that a caller might use less caution for the urls argument if launching to third party content is intended.

(Just putting -- in front of the URLs feeels like it's only protecting against a specific use-case, instead of the general problem.)

I'd call it a solution well tailored for the use case. The API we own here is a slim wrapper on the chrome executable, and this change is IMO a more valid translation from Dart to chrome CLI. It's questionable whether we ever should have put a distinction between arguments and URLs in the Dart API, given that the caller is anyways making this call with the domain knowledge of the -- CLI flags. Given that we do have separate args and urls in Dart API, using -- between them is a better translation to the CLI API, and any deeper validation is rightly handled by chrome.