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

[2.19] Document content-hashing in pubspec.lock #4361

Closed
jonasfj opened this issue Nov 11, 2022 · 10 comments · Fixed by #4407
Closed

[2.19] Document content-hashing in pubspec.lock #4361

jonasfj opened this issue Nov 11, 2022 · 10 comments · Fixed by #4407
Assignees
Labels
co.request Community ask for documentation dev.packages Relates to package publishing e1-hours Can complete in < 8 hours of normal, not dedicated, work p1-high Major but not urgent concern: Resolve in months. Update each month.

Comments

@jonasfj
Copy link
Member

jonasfj commented Nov 11, 2022

Starting Dart 2.19 we're embedding a sha256 content-hashes in pubspec.lock.

We should document this, and create a link dart.dev/go/content-hashes pointing to the documentation. This link have already been added to messages in the pub tool and sdk CHANGELOG.

@jonasfj jonasfj added the dev.packages Relates to package publishing label Nov 11, 2022
@jonasfj
Copy link
Member Author

jonasfj commented Nov 11, 2022

Filed link in #4362

@sigurdm
Copy link
Contributor

sigurdm commented Nov 11, 2022

The feature was implemented here: dart-lang/pub#3482

@parlough parlough added p2-medium Necessary but not urgent concern. Resolve when possible. e1-hours Can complete in < 8 hours of normal, not dedicated, work co.request Community ask for documentation labels Nov 11, 2022
parlough pushed a commit that referenced this issue Nov 11, 2022
Ensuring that the link doesn't go to 404, instead it'll go to the bug
that we should write the page.

See #4361

Hopefully, this helps us not forget to write the page, and helps users
highlight it if we do.
It's certainly better than having a 404.
@parlough parlough added p1-high Major but not urgent concern: Resolve in months. Update each month. and removed p2-medium Necessary but not urgent concern. Resolve when possible. labels Nov 11, 2022
@parlough
Copy link
Member

parlough commented Nov 15, 2022

Without the new pub get option implemented (dart-lang/pub#2890), what should be documented here? The fact that a sha256 hash of the package archive is included in the pubspec.lock description of each package that can be manually verified against?

We don't really have great centralized documentation of pubspec.lock. Perhaps the best place is the Lockfile section of Package versioning.

@johnpryan
Copy link
Contributor

From a user perspective, how do you use this feature to verify the integrity of downloaded packages?

@MaryaBelanger MaryaBelanger mentioned this issue Nov 29, 2022
6 tasks
@MaryaBelanger
Copy link
Contributor

We don't really have great centralized documentation of pubspec.lock. Perhaps the best place is the Lockfile section of Package versioning.

That seems like the only decent place to put it at the moment. It might be a little too high level/conceptual if we want to document the "how" in @johnpryan's question (@jonasfj or @sigurdm it'd be wonderful to get that info to fill out the docs!)

Another option:
Currently the pubspec page is just about pubspec.yaml, but maybe that'd be a good place to tack a pubspec.lock section at the end to cover the more technical/implementation side of pubspec.lock? That would also give us a good place to talk about the technical implementation of --enforce-lockfile that's also going into 2.19

An aside:
Isn't it odd that neither the dart pub get, upgrade, or downgrade pages mention pubspec.lock at all, even though that's how the file gets created?

@jonasfj
Copy link
Member Author

jonasfj commented Nov 30, 2022

Currently the pubspec page is just about pubspec.yaml, but maybe that'd be a good place to tack a pubspec.lock section

Maybe... I'm inclined to keep the "pubspec page" being just reference documentation. It's the closest thing to a specification we have for pubspec.yaml.


We could also mention --enforce-lockfile in the best practices section: https://dart.dev/tools/pub/dependencies#best-practices


From a user perspective, how do you use this feature to verify the integrity of downloaded packages?

@johnpryan, when calling dart pub get --enforce-lockfile it'll refuse the resolve the hashes (or anything) in the lock file don't match.

@MaryaBelanger MaryaBelanger self-assigned this Nov 30, 2022
@MaryaBelanger MaryaBelanger changed the title Document content-hashing in pubspec.lock [2.19] Document content-hashing in pubspec.lock Nov 30, 2022
@MaryaBelanger
Copy link
Contributor

@sigurdm Question about these two statements that seem to be conflicting:

Here in (I'm assuming) the API docs:

In the absense of this field the client can still download the archive to obtain a checksum and detect changes to the archive.

And here in the feature PR:

... won't add hashes if missing,

Does this mean that

  1. the client can download an archive,
  2. and if the downloaded package doesn't have a hash, the client will compute one from its contents
  3. then the client will use that newly computed hash for comparison,
  4. and then discard the hash when it's done verifying against the cached package?

If I'm wrong, could you explain what each of these points mean in reference to each other? Thank you

@sigurdm
Copy link
Contributor

sigurdm commented Dec 6, 2022

I can try:

The version-listing API optionally can serve the hash of the archive. This can be used to verify the downloaded archive is correct. pub.dev implements this.

No matter if the hash is in the version listing or not it will be computed after download of the archive, and stored in the pub cache for later reference. (I think this is what you were missing).

Now a hash in an existing pubspec.lock will first be verified against the corresponding one in the pub cache (if it exists). And if there's a mis-match the package will get redownloaded (under the assumption that the pub cache is wrong)

If redownloading doesn't help (the locked hash is actually wrong) the pubspec.lock will be updated (under the assumption that the pubspec.lock was outdated or that the server was updated, but in a benign way) - a warning is written, as this should happen very rarely if ever. We could have chosen to give an error here - but that would leave the user with no other real choice than hand-editing or deleting the lockfile.

But sometimes you really want to be sure that you get exactly what you ask for, and that is what --enforce-lockfile can do for you.

the client can download an archive, and if the downloaded package doesn't have a hash, the client will compute one from its contents

No - it will always compute it and store in the cache. Only if it is missing from the version-listing response it cannot validate the download.

then the client will use that newly computed hash for comparison, and then discard the hash when it's done verifying against the cached package?

No it is stored in the pub cache alongside the extracted package.

@MaryaBelanger
Copy link
Contributor

Thank you @sigurdm that makes much more sense, really appreciate your time!

atsansone pushed a commit that referenced this issue Dec 20, 2022
Fixes #4361 

Changes:

- Added a best practices entry about `--enforce-lockfile` to
[pub/dependencies](https://dart.dev/tools/pub/dependencies#best-practices)
page (per @jonasfj suggestion
[here](#4361 (comment)))
- Added `--enforce-lockfile` to
[pub-get#options](https://dart.dev/tools/pub/cmd/pub-get#options)
section
- Adjusted the
[pub/glossary#lockfile](https://dart.dev/tools/pub/glossary#lockfile)
section to discuss content hashing
- Adjusted the
[pub/versioning#lockfiles](https://dart.dev/tools/pub/versioning#lockfiles)
section to discuss content hashing (per @parlough suggestion
[here](#4361 (comment)))

I'm fairly confident these are the right _places_ to add info on these
new features, but am not confident in the accuracy/quality of the
information I added. I think I might've conflated what content hashes
actually do vs what the `--enforce-lockfile` flag does.

Looking forward to reviews to get this right!

Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>
Co-authored-by: Parker Lougheed <parlough@gmail.com>
@MaryaBelanger
Copy link
Contributor

Fixing PRs merged into v2.19

parlough added a commit that referenced this issue Jan 23, 2023
Fixes #4361 

Changes:

- Added a best practices entry about `--enforce-lockfile` to
[pub/dependencies](https://dart.dev/tools/pub/dependencies#best-practices)
page (per @jonasfj suggestion
[here](#4361 (comment)))
- Added `--enforce-lockfile` to
[pub-get#options](https://dart.dev/tools/pub/cmd/pub-get#options)
section
- Adjusted the
[pub/glossary#lockfile](https://dart.dev/tools/pub/glossary#lockfile)
section to discuss content hashing
- Adjusted the
[pub/versioning#lockfiles](https://dart.dev/tools/pub/versioning#lockfiles)
section to discuss content hashing (per @parlough suggestion
[here](#4361 (comment)))

I'm fairly confident these are the right _places_ to add info on these
new features, but am not confident in the accuracy/quality of the
information I added. I think I might've conflated what content hashes
actually do vs what the `--enforce-lockfile` flag does.

Looking forward to reviews to get this right!

Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>
Co-authored-by: Parker Lougheed <parlough@gmail.com>
parlough added a commit that referenced this issue Jan 23, 2023
Fixes #4361

Changes:

- Added a best practices entry about `--enforce-lockfile` to
[pub/dependencies](https://dart.dev/tools/pub/dependencies#best-practices)
page
- Added `--enforce-lockfile` to
[pub-get#options](https://dart.dev/tools/pub/cmd/pub-get#options)
section
- Adjusted the
[pub/glossary#lockfile](https://dart.dev/tools/pub/glossary#lockfile)
section to discuss content hashing
- Adjusted the
[pub/versioning#lockfiles](https://dart.dev/tools/pub/versioning#lockfiles)
section to discuss content hashing

Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>
Co-authored-by: Parker Lougheed <parlough@gmail.com>
parlough added a commit that referenced this issue Jan 24, 2023
Fixes #4361

Changes:

- Added a best practices entry about `--enforce-lockfile` to
[pub/dependencies](https://dart.dev/tools/pub/dependencies#best-practices)
page
- Added `--enforce-lockfile` to
[pub-get#options](https://dart.dev/tools/pub/cmd/pub-get#options)
section
- Adjusted the
[pub/glossary#lockfile](https://dart.dev/tools/pub/glossary#lockfile)
section to discuss content hashing
- Adjusted the
[pub/versioning#lockfiles](https://dart.dev/tools/pub/versioning#lockfiles)
section to discuss content hashing

Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>
Co-authored-by: Parker Lougheed <parlough@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
co.request Community ask for documentation dev.packages Relates to package publishing e1-hours Can complete in < 8 hours of normal, not dedicated, work p1-high Major but not urgent concern: Resolve in months. Update each month.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants