Skip to content

feat: Add Checksums to package.json format#1217

Merged
kodiakhq[bot] merged 5 commits intomainfrom
checksums
Sep 18, 2023
Merged

feat: Add Checksums to package.json format#1217
kodiakhq[bot] merged 5 commits intomainfrom
checksums

Conversation

@hermanschaaf
Copy link
Copy Markdown
Contributor

Adds zip file checksums to the package.json format.

Includes some commits from #1216 to avoid merge conflicts later.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 18, 2023

⏱️ Benchmark results

Comparing with 70f20bb

  • Glob-8 ns/op: 99.51 ⬇️ 0.79% decrease vs. 70f20bb

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 18, 2023

Codecov Report

Patch coverage: 54.54% and project coverage change: +0.02% 🎉

Comparison is base (70f20bb) 48.51% compared to head (00e5cc5) 48.54%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1217      +/-   ##
==========================================
+ Coverage   48.51%   48.54%   +0.02%     
==========================================
  Files          87       87              
  Lines        8142     8185      +43     
==========================================
+ Hits         3950     3973      +23     
- Misses       3828     3842      +14     
- Partials      364      370       +6     
Files Changed Coverage Δ
serve/package.go 55.41% <54.54%> (-0.44%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

OS string `json:"os"`
Arch string `json:"arch"`
Path string `json:"path"`
Checksum string `json:"checksum"`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How are we going to use the Checksum? Shouldn't we also sign the checksum file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please note that also with our current deployment tool (GoReleaser) we have checksums but we don't sign them

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@erezrokah they will be sent with HTTP PATCH after the upload.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@erezrokah they will be sent with HTTP PATCH after the upload.

Thanks @disq, but what do we do with them? Do we validate it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@erezrokah We pass them on to the client and after download the client should validate them (soon)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How are we going to use the Checksum?

We will store it in package.json here temporarily, and then read and upload it along with the binaries via the cloudquery publish command (which reads the package.json file). Then it'll be stored on the backend, and used to verify downloads of binaries from the CLI.

Shouldn't we also sign the checksum file

I think we can add this on the backend at a later stage. The only case I can think of is if an attacker were to do a MITM-type attack between us and the user downloading the plugin. To mitigate this I think the CLI will need to ship with our public key, to make sure the checksum file came from us.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, mostly mentioning since we don't do any kind of checksum validation at the moment so maybe we can drop it altogether

@kodiakhq kodiakhq bot merged commit 720baae into main Sep 18, 2023
@kodiakhq kodiakhq bot deleted the checksums branch September 18, 2023 15:15
kodiakhq bot pushed a commit that referenced this pull request Sep 19, 2023
🤖 I have created a release *beep* *boop*
---


## [4.8.0](v4.7.1...v4.8.0) (2023-09-19)


### Features

* Add Checksums to package.json format ([#1217](#1217)) ([720baae](720baae))
* Add message to package command ([#1216](#1216)) ([44956d9](44956d9))
* Add shuffle scheduler ([#1218](#1218)) ([2b1ba30](2b1ba30))
* Update package command ([#1211](#1211)) ([39fc65e](39fc65e))


### Bug Fixes

* Add schema version to package.json ([#1212](#1212)) ([393c94d](393c94d))
* **deps:** Update github.com/cloudquery/arrow/go/v14 digest to 483f6b2 ([#1209](#1209)) ([179769a](179769a))
* **deps:** Update github.com/cloudquery/arrow/go/v14 digest to ffb7089 ([#1215](#1215)) ([70f20bb](70f20bb))
* Use -dir suffix for plugin package arguments ([#1213](#1213)) ([93f9398](93f9398))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants