pub icon indicating copy to clipboard operation
pub copied to clipboard

A way to restrict the allowed hosts for dependencies.

Open matanlurey opened this issue 1 year ago • 5 comments

Context: flutter/engine prohibits dependencies from pub, requiring all packages to come from gclient sync (i.e. our Chromium version of git submodules), so we go out of our way to make sure that, given a repo xyz, package xyx/packages/foo doesn't start depending on say, package:archive without adding it to xyz/DEPS.

In https://github.com/flutter/engine/pull/53539#discussion_r1681401047, @zanderso asked:

For my understanding, is it possible for a pubspec that uses the workspace to add a dependency on a package that isn't in the workspace? Like can a workspace-using pubspec add a new dependency and pull a package from pub that isn't declared in the workspace?

It looks like the answer was surprisingly yes:

# Declare all packages that are part of the workspace.
workspace:
  - tools/engine_tool

dependencies:
  args: any

dependency_overrides:
  args:
    path: ./third_party/dart/third_party/pkg/args

And then later, in tools/engine_tool/pubspec.yaml:


name: engine_tool

# This package is managed as part of the engine workspace.
resolution: workspace

dependencies:
  archive: any
  args: any

I expected this to fail, or warn, or do something. Here is the result of dart pub get:

Resolving dependencies in `/Users/matanl/Developer/engine/src/flutter`... 
Downloading packages... 
! args 2.5.1-wip from path ../../third_party/dart/third_party/pkg/args (overridden)
! async 2.12.0-wip from path ../../third_party/dart/third_party/pkg/async (overridden)
! async_helper 0.0.0 from path ../../third_party/dart/pkg/async_helper (overridden)
! collection 1.19.0 from path ../../third_party/dart/third_party/pkg/collection (overridden)
! engine_build_configs 0.0.0 from path ../pkg/engine_build_configs (overridden)
! engine_repo_tools 0.0.0 from path ../pkg/engine_repo_tools (overridden)
! expect 0.0.0 from path ../../third_party/dart/pkg/expect (overridden)
! file 7.0.1-dev from path ../../third_party/dart/third_party/pkg/file/packages/file (overridden)
! litetest 0.0.0 from path ../../testing/litetest (overridden)
! logging 1.3.0-wip from path ../../third_party/dart/third_party/pkg/logging (overridden)
! meta 1.16.0-dev from path ../../third_party/dart/pkg/meta (overridden)
! path 1.9.1-wip from path ../../third_party/dart/third_party/pkg/path (overridden)
! platform 3.1.0 from path ../../third_party/pkg/platform (overridden)
! process 4.2.1 from path ../../third_party/pkg/process (overridden)
! process_fakes 0.0.0 from path ../pkg/process_fakes (overridden)
! process_runner 4.1.3 from path ../../third_party/pkg/process_runner (overridden)
! smith 0.0.0 from path ../../third_party/dart/pkg/smith (overridden)
! source_span 1.10.1-wip from path ../../third_party/dart/third_party/pkg/source_span (overridden)
! string_scanner 1.3.0 from path ../../third_party/dart/third_party/pkg/string_scanner (overridden)
! term_glyph 1.2.2-wip from path ../../third_party/dart/third_party/pkg/term_glyph (overridden)
! yaml 3.1.3-wip from path ../../third_party/dart/third_party/pkg/yaml (overridden)
Got dependencies in `/Users/matanl/Developer/engine/src/flutter`!

Note that archive is omitted, and not mentioned. However, if I look at the root .dart_tool/package_config.json:

{
  "configVersion": 2,
  "packages": [
    {
      "name": "archive",
      "rootUri": "file:///Users/matanl/.pub-cache/hosted/pub.dev/archive-3.6.1",
      "packageUri": "lib/",
      "languageVersion": "3.0"
    },
    {
      "name": "args",
      "rootUri": "../third_party/dart/third_party/pkg/args",
      "packageUri": "lib/",
      "languageVersion": "3.3"
    }
}

... and I can indeed import and use package:archive in engine_tool. This is not a blocker for us as we have that custom script that checks that packages aren't resolved to pub, but it was surprising and silent behavior - I'd love to see some mechanism to get "protection" or at least notice that this is happening.


Possible suggestions for improvement:

  • [ ] Clarify how the tool is intended to work to make sure the above is expected
  • [ ] Ensure package:archive comes up in dart pub get and similar, perhaps with a (not from workspace) moniker?
  • [ ] Allow the root pubspec to have something like workspace_options: { "dependency_sources": "workspace_only" }
  • [ ] Add a lint, pub_workspace_single_source_of_dependencies that can be opted-in to

/cc @kevmoo @jtmcdole

matanlurey avatar Jul 17 '24 17:07 matanlurey

Dependencies from workspace-only is likely a niche use case.

When doing things like this you are effectively saying that you don't want to use the version resolution logic that pub provides. Instead you're just using pub to generate a package_config.json and get a development experience that is somewhat similar to normal app developers who gets packages from pub.dev.


We could certainly add things to pubspec.yaml that would make setups like this nicer. Like an allowed_sources as a list of git-repos, pub-servers and paths that are allowed as package sources in the resolution of a pubspec (only when the pubspec is a root, like how dev-dependencies work).

The allowed_sources idea above is just a quick of the top of my head. I think modt people won't need it. And I think that advanced users could solve by just writing a test case that reads pubspec.lock to check where dependencies comes from.

I think a feature like allowed_sources will be relevant when/if we add support for SLSA in pub. Then you might want to specify that you only want dependencies thet satisfy certain constraints like:

  • An allowed list of publishers
  • Requirement for SLSA level 2, 3 or ?,
  • Enforcement that no version may be affected by a security vulnerability.
  • An allow list of hosted repositories
  • Packages must be published using automated publishing... Etc..

I think that once we have SLSA support on pub.dev we should spend time designing a set of policies to be enforced on dependencies. I guess workspace feature is a good mechanism for enforcing such policies across a mono-repository.

TL;DR: Some sort of policy enforcement on dependencies would be cool, but maybe we shouldn't design it until we have SLSA. In the meantime people can enforce policies with a test case.


One could also imagine something like hook/preresolve.dart, a script that is called before pub get resolve. That could then trigger gclient sync. But that might make pub get so slow that you can't run it from an IDE whenever you modify a relevant file.

jonasfj avatar Jul 17 '24 20:07 jonasfj

To be clear, the main surprise was that it silently works and doesn't even show up in the pub get list.

matanlurey avatar Jul 17 '24 20:07 matanlurey

I think we just want a failure at pub get with package {package referenced} not defined at workspace pubspec.yaml.

jtmcdole avatar Jul 17 '24 22:07 jtmcdole

Dependencies from workspace-only is likely a niche use case.

I do actually think that for larger companies this might not be a niche use case. It is probably not all that uncommon to want to have some higher level control over exactly what dependencies are allowed, where they come from, and exactly what version a project is on.

jakemac53 avatar Jul 19 '24 17:07 jakemac53

I like the idea of being able to restrict dependencies to a number of select hosts. I guess that should only take effect in the root pubspec...

allowed_host_urls:
  - https://pub.dev
  - https://my-pub-server.com

and for the flutter engine you would provide an empty list:

allowed_host_urls: [] # don't allow any hosted packages

Maybe jonas is right and we should also allow for specifying allowed publishers, salsa levels etc. But I think these are somewhat orthogonal, and could be added separately later...

sigurdm avatar Aug 01 '24 13:08 sigurdm