Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent submitting packages with hook/xx.dart that are not allowlisted #7541

Open
dcharkes opened this issue Mar 7, 2024 · 8 comments
Open

Comments

@dcharkes
Copy link

dcharkes commented Mar 7, 2024

@jonasfj suggested that we should prevent people from uploading packages to pub.dev with hooks that we haven't specified.

This should be relatively easy to add to the validator.

For now, we'd only allowlist hook/build.dart and hook/link.dart.

Context:

cc @mosuem @sigurdm

@jonasfj jonasfj transferred this issue from dart-lang/pub Mar 7, 2024
@jonasfj
Copy link
Member

jonasfj commented Mar 7, 2024

I think we should at-least consider it. Probably we need to scan all packages to see if anyone is already using this directory.

@sigurdm
Copy link
Contributor

sigurdm commented Mar 7, 2024

I guess it would be an overridable warning. Another pub client-side validation?

Or do we want a server-side check?

@mosuem
Copy link
Member

mosuem commented Mar 8, 2024

What about helper files, if my build.dart is getting too complex? Should they not be stored in hooks/src/ or similar?

@dcharkes
Copy link
Author

What about helper files, if my build.dart is getting too complex? Should they not be stored in hooks/src/ or similar?

I'd probably put these in src/. I think for bin/ scripts we also store the complex logic in src/. The bin/ and hook/ directories should only contain the entry-points. cc @jonasfj @sigurdm

@sigurdm
Copy link
Contributor

sigurdm commented Mar 14, 2024

Is this language versioned? (will future sdk's potentially allow more files here)?

@sigurdm
Copy link
Contributor

sigurdm commented Mar 14, 2024

I think we are converging on a server-side check. That is easier to update/relax over time.

@jonasfj
Copy link
Member

jonasfj commented Mar 14, 2024

Let's make a server side map:

final _hookMinimumRequiresSdkForFile = {
  // because if you have "hook/build.dart" in the package
  // then you must have 'environment.sdk' with a minimum bound of 3.45.0
  // The actual version number is illustrative.
  'build.dart': '3.45.0',
};

@dcharkes
Copy link
Author

I think we are converging on a server-side check. That is easier to update/relax over time.

Ah right, because we can just deploy a new version. With a client-side check we have old versions of Dart/pub client floating around forever. (Though I guess users could never submit a package with a newer version of Dart than the SDK they have locally.)

You're the experts, sgtm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants