tidbyt / pixlet

Build apps for pixel-based displays ✨
https://tidbyt.com
Apache License 2.0
756 stars 107 forks source link

Validate appID before encrypting #1072

Open dinosaursrarr opened 4 months ago

dinosaursrarr commented 4 months ago

We've all done it. It's too easy to use the wrong key to encrypt with, whether you use the app name, the folder name, or get the hyphens wrong. Cropped up again recently with https://github.com/tidbyt/pixlet/issues/1071.

If you're running pixlet encrypt from within the community repo, this will add a check that the provided key matches the ID field from some existing manifest. If not, you get an error.

Thought about @jmanske's suggestions but I don't think pixlet can assume there'll be exactly one manifest in the commit to check against. You might be updating multiple apps, in which case, which should you check? Or you might be fixing a bad encryption, and not have any local edits. App names are generally unique enough that I think this works.

rohansingh commented 4 months ago

Looks good. I need to test it with private apps (which can also use encryption) to make sure it works there.

dinosaursrarr commented 4 months ago

My other idea was to change the cli to expect a path to a manifest file, rather than an appID string. Then you know exactly where to extract the appID in any environment. The only reason I didn't do that was to avoid breaking anyone by changing the interface.

jmanske commented 3 months ago

We could also have a "single arg" version of encrypt that assumes you are in your app folder and errors if there is no manifest file. Read the manifest file and pull the id. This is far and away the main use case. Also helps with usability in private apps when the ID is some UUID.

Since min args right now is set to 2 this doesn't break the interface since this would be pure enhancement.