aklinker1 / vite-plugin-web-extension

Vite plugin for developing Chrome/Web Extensions
https://vite-plugin-web-extension.aklinker1.io/
MIT License
574 stars 48 forks source link

Custom JSON validation formats for manifest #2

Closed aklinker1 closed 1 year ago

aklinker1 commented 2 years ago

Right now, some fields are not validated, any fields that reference these types just accept any string:

Here's the schema for V3: https://json.schemastore.org/chrome-manifest

They should perform actual validation checks, not just checking if they're a string.

Here's where we're setting these formats to accept any string.

https://github.com/aklinker1/vite-plugin-web-extension/blob/b2ca582c426132ae4ef5b9600e047b94c13fd8c4/packages/vite-plugin-web-extension/src/manifest-validation.ts#L23-L28

karmaral commented 1 year ago

Hello! Is this still needed? The schema seems to have all of those now, only difference being they use underscores.

aklinker1 commented 1 year ago

Yes, this is still needed. For example, the validation wouldn't catch if you passed asdf as a permission.

Screen Shot 2023-03-29 at 7 05 02 PM

The "permission" format is missing. The validation does not compile unless we add the format ourselves:

https://github.com/aklinker1/vite-plugin-web-extension/blob/b2ca582c426132ae4ef5b9600e047b94c13fd8c4/packages/vite-plugin-web-extension/src/manifest-validation.ts#L24

aklinker1 commented 1 year ago

If we comment out any of those lines, we get an error like this:

Screen Shot 2023-03-29 at 7 07 23 PM
karmaral commented 1 year ago

I see, yeah. Might be worth fixing it in the source repo. I'll dig in a bit deeper.

aklinker1 commented 1 year ago

Yeah. Ideally the format would be defined in the schema itself, somehow. I'm not too well versed in JSON schemas, so I don't even know if you can define custom formats inside a schema.

karmaral commented 1 year ago

So from what I'm gathering, these formats might be there as placeholders, since they're not defined anywhere else in the schema and are even flagged as unknown in the schema-validation file.

Now, you can define custom formats, but I don't quite see why you would want to in this case, since most of it can be fixed by using patterns or enums.

I already updated the permissions to behave apropiately so it should bounce off bogus keywords or improper match_patterns.

Removing these redundant formats, along with the permissions update would solve most of these errors. I'm still investigating the content_security_policy validation and the glob_pattern, the latter being apparently just a string as there's not really a way to 'clamp' it inside a regex.

However: Checking the webextension manifest and its docs, there seem to be a few permission keywords that are not contained in Chrome's enum.

Do you think it's best if we compare against each manifest depending on the target build and trim the unwanted bits behind the scenes, or just stick to the Chrome one and just do the casting entirely within the plugin?

I'm not familiar yet with how you're handling the validations internally as I've mostly been digging in the schemastore repo, but the multi-schema sounds like the most orderly way of doing it. I'd hope it doesn't explode into complexity though 😂.

aklinker1 commented 1 year ago

Do you think it's best if we compare against each manifest depending on the target build and trim the unwanted bits behind the scenes, or just stick to the Chrome one and just do the casting entirely within the plugin?

The build target isn't necessarily going to be chrome, firefox, or safari. For example, one of my extensions includes an ios target. We wouldn't know which validation format to use for that target.

We could use multiple schemas if we added another option for the schema target, and make some assumptions based on the existing target.

Right now, I'm just using the chrome schema, and I provide the option to skip validation.

All the validation logic is in the file liked in the issue description.

aklinker1 commented 1 year ago

I'm going to close this issue, there doesn't appear to be any solution, event after 2 years.