Closed Bug 1229197 Opened 4 years ago Closed 4 years ago

Allow unlisted add-ons to be signed without passing the validator

Categories

(addons.mozilla.org Graveyard :: Add-on Validation, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: andy+bugzilla, Assigned: magopian)

Details

(Whiteboard: [qa+])

If an add-on is:
* unlisted
* NOT side-loaded
* NOT a web extension
* and the validation fails without a critical error [1]

Then do not prevent the add-on being submitted and signed, let it continue.

Hoping we can get this through for our Thursday 3rd push. Making confidential for the moment, until Kev does the communication.

[1] The most critical errors are things like: 
- version already exists
- validation timeout
- version doesn't match
- could not parse manifest
- could not find add-on ID
- add-on id doesn't match add-on
- duplicate add-on id
- Version numbers should have fewer than 32 characters
- Version numbers should only contain letters, numbers, and these punctuation characters: +*.-_. - <em:type> doesn't match add-on
or put another way "I think it should be everything in tier 1 and the errors from files.utils.parse_addon which seem to end up on the Form objects".
We can drop the "NOT side-loaded" requirement. As per feedback from Jorgev. Side-loaded add-ons still need to be manually enabled by the user in the add-ons manager.

So side-loaded unlisted addons should also be signed by the validator if they do not fail critically.
This will need a whole lot of QA: making sure there's no issues
1/ submitting new add-ons listed, unlisted, unlisted and sideload, with or without validation errors
2/ submitting new versions for add-ons
3/ submitting new files for versions
4/ submitting files to the standalone validator
Assignee: nobody → magopian
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Whiteboard: [qa+]
Group: mozilla-employee-confidential
Thanks, everyone. Where does this leave extensions that currently time out the validator, though? If the other critical errors are prioritized in the tests, could the output leading up to the timeout be sufficient?
(In reply to Dan Stillman from comment #4)
> Thanks, everyone. Where does this leave extensions that currently time out
> the validator, though? If the other critical errors are prioritized in the
> tests, could the output leading up to the timeout be sufficient?

That's a way to go, but would be change outside of the scope of this change. At this point it would mean some critical tests have failed, but perhaps we can seperate that out. 

For the record we've upped the timeout on the validator in bug 1229060 and I tested zotero along with other add-ons to see if I could get a timeout and so have not triggered one with the new timeout.
(In reply to Andy McKay [:andym] from comment #5)
> (In reply to Dan Stillman from comment #4)
> > Thanks, everyone. Where does this leave extensions that currently time out
> > the validator, though? If the other critical errors are prioritized in the
> > tests, could the output leading up to the timeout be sufficient?
> 
> That's a way to go, but would be change outside of the scope of this change.
> At this point it would mean some critical tests have failed, but perhaps we
> can seperate that out. 
> 
> For the record we've upped the timeout on the validator in bug 1229060 and I
> tested zotero along with other add-ons to see if I could get a timeout and
> so have not triggered one with the new timeout.

Sorry. Typo there "So far have not triggered a timeout one those add-ons with the new timeout setting."
I went through the errors that we register and everything that we call an error seems to be something that we'd want to block signing. So tier doesn't matter after all.

As Andy mentioned timeouts would fail, we register them as errors. It could be that we don't register any errors after a certain tier (it looks like 1 and 2 definitely have errors) but we don't know what tier we failed on and that gets interesting for inner packages since they handle tiers differently. I don't think we know enough about timeouts and I'm not sure we could know enough.

I've got a PR [1] to handle this in the signing API. Should merge it and get it into the tag tomorrow.

[1] https://github.com/mozilla/olympia/pull/1014
QA Contact: vasilica.mihasca
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.