chore: sign the vsix package, not the manifest #83
Conversation
| // If this file is missing, it means the extension was added before | ||
| // signatures were handled by the marketplace. |
There was a problem hiding this comment.
Looks like this opens the .vsix, which should always exist (if not installing will also fail). Or I could be misreading this 😅
There was a problem hiding this comment.
Wait no, the manifest is expected to be there. Now that we are signing the vsix though, I might change this later to not require the file to be there.
There was a problem hiding this comment.
This comment refers to loading the vsix though, not the manifest, and the vsix must always exist. Possibly it was supposed to go in the block above where we read the manifest?
There was a problem hiding this comment.
Oh yeah probably, the todo and error below also says manifest instead of vsix :D
| } | ||
|
|
||
| // TODO: Fetch the VSIX payload from the storage | ||
| signed, err := s.SigZip(ctx, vsixData, manifestData) | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("sign and zip manifest: %w", err) |
There was a problem hiding this comment.
Oops meant to say before submitting, really big nit, but looks like this error would end up saying sign and zip manifest: sign and zip manifest: whatever the error is with the first part twice which could look weird
I misread the nodejs implementation, see this line: filepath.Join(filepath.Dir(fp), vsixPath+".vsix")
Should be signing the vsix file.
Regardless, signatures are still not being checked 🤷♂️. Unsure why VSCode just accepts bad signatures 🤔.
Also adds a command line flag to save the signatures to disk. Really nice for debugging.