Changes to wintenapps for testing

[nvdaes]

No description provided.

[nvdaes]

Hi @feerrenrut and @josephsl
Previously I have stuck. But I think that automerge can be a security risk for now. I've changed data replacing Wintenapps and the check pass. Also I'm thinking: what if I would like to add an add-on with name wintenapps, malicious code and hosting it on my GitHub account and I changeeven the sha256 and the download repo replacing the add-on with the same id, so that metadata matches perfectly, changing the download url?
I think it's better to create issues so that publishers are something that cannot be changed. So that add-ons can be distinguished not just by ids and other metadata but also by publishers, set automatically from the person who sends the add-on from an issue.
The addon-datastore-validation may have a Python file to create many data stored in a json file automatically. If I change stable to beta in Joseph's add-on, this can be harmful since people may not see it if they don't want to show prereleases etc.

[nvdaes]

i may test on my GitHub account my proposed implementation to send issues which set the publisher automatically, unless you reject this approach. I"ll start these tests today, I think, after job.

[seanbudd]

Perhaps we could maintain a list of publishers (GitHub accounts) with auto-approval, and other PRs require manual approval.

[nvdaes]

Hello, thanks for your valuable work. I"m experimenting basing a workflow on a GitHub workflow created by you on the transform repo. I"m trying to download the add-on from an issue form, providing the url, and then generating the json file from the manifest, with getManifest(addonDestPath) method, checking out the add-on-store-validation repo. I"m thinking about creating a separate py file with getManifest, similar to sha256.py. Your idea is good, though the problem is how to decide the trusted accounts. Enviado desde mi iPhone

…

El 27 ene 2022, a las 4:00, Sean Budd ***@***.***> escribió:  Perhaps we could maintain a list of publishers (GitHub accounts) with auto-approval, and other PRs require manual approval. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

[nvdaes]

For reference, I'll try to use this package: https://pythonrepo.com/repo/ghandic-jsf-python-josn 2022-01-27 4:08 GMT+01:00, Noelia Ruiz Martínez ***@***.***>:

…

Hello, thanks for your valuable work. I"m experimenting basing a workflow on a GitHub workflow created by you on the transform repo. I"m trying to download the add-on from an issue form, providing the url, and then generating the json file from the manifest, with getManifest(addonDestPath) method, checking out the add-on-store-validation repo. I"m thinking about creating a separate py file with getManifest, similar to sha256.py. Your idea is good, though the problem is how to decide the trusted accounts. Enviado desde mi iPhone > El 27 ene 2022, a las 4:00, Sean Budd ***@***.***> escribió: > >  > Perhaps we could maintain a list of publishers (GitHub accounts) with > auto-approval, and other PRs require manual approval. > > — > Reply to this email directly, view it on GitHub, or unsubscribe. > You are receiving this because you authored the thread. -- Reply to this email directly or view it on GitHub: #35 (comment) You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

[nvdaes]

Finally I've used a VALID_JSONFILE constant referencing the valid json file contained in test data. Running the runcreatejson.bat file an output/output.json file with data modified from manifest seems to be properly generated. I've modified the addon-datastore-validation repo on my own fork, main branch, to test better with GitHub Actions. I've also performed tests locally. I expect to modify a GitHub workflow (.yaml file) in my fork of addon-datastore to test with an issue sending just the sourceUrl and the hannel, since other field should be retrieved from manifest or sha256.py. I comment each milestone in case you prefer to go in a different direction. 2022-01-27 7:40 GMT+01:00, Noelia Ruiz Martínez ***@***.***>:

…

For reference, I'll try to use this package: https://pythonrepo.com/repo/ghandic-jsf-python-josn 2022-01-27 4:08 GMT+01:00, Noelia Ruiz Martínez ***@***.***>: > Hello, thanks for your valuable work. I"m experimenting basing a workflow > on > a GitHub workflow created by you on the transform repo. I"m trying to > download the add-on from an issue form, providing the url, and then > generating the json file from the manifest, with > getManifest(addonDestPath) > method, checking out the add-on-store-validation repo. I"m thinking > about > creating a separate py file with getManifest, similar to sha256.py. Your > idea is good, though the problem is how to decide the trusted accounts. > > Enviado desde mi iPhone > >> El 27 ene 2022, a las 4:00, Sean Budd ***@***.***> escribió: >> >>  >> Perhaps we could maintain a list of publishers (GitHub accounts) with >> auto-approval, and other PRs require manual approval. >> >> — >> Reply to this email directly, view it on GitHub, or unsubscribe. >> You are receiving this because you authored the thread. > > > -- > Reply to this email directly or view it on GitHub: > #35 (comment) > You are receiving this because you are subscribed to this thread. > > Message ID: ***@***.***> -- Reply to this email directly or view it on GitHub: #35 (comment) You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

[nvdaes]

Hello, finally I have created an issue form on my addon-datastore fork for testing. I've checked that sending the URL and source url, and checking the stable checkbox, eMule add-on is downloaded automatically, the json file is generated and sent to master for now. When the file exists, a ValueError exception is raised to prevent overriding. I think it's better to create PRs instead of sending add-ons diretly, and work needs to be done to request removals, and also to close issues when pull requests are merged: https://github.com/nvdaes/addon-datastore/issues/new/choose 2022-01-27 7:40 GMT+01:00, Noelia Ruiz Martínez ***@***.***>:

…

For reference, I'll try to use this package: https://pythonrepo.com/repo/ghandic-jsf-python-josn 2022-01-27 4:08 GMT+01:00, Noelia Ruiz Martínez ***@***.***>: > Hello, thanks for your valuable work. I"m experimenting basing a workflow > on > a GitHub workflow created by you on the transform repo. I"m trying to > download the add-on from an issue form, providing the url, and then > generating the json file from the manifest, with > getManifest(addonDestPath) > method, checking out the add-on-store-validation repo. I"m thinking > about > creating a separate py file with getManifest, similar to sha256.py. Your > idea is good, though the problem is how to decide the trusted accounts. > > Enviado desde mi iPhone > >> El 27 ene 2022, a las 4:00, Sean Budd ***@***.***> escribió: >> >>  >> Perhaps we could maintain a list of publishers (GitHub accounts) with >> auto-approval, and other PRs require manual approval. >> >> — >> Reply to this email directly, view it on GitHub, or unsubscribe. >> You are receiving this because you authored the thread. > > > -- > Reply to this email directly or view it on GitHub: > #35 (comment) > You are receiving this because you are subscribed to this thread. > > Message ID: ***@***.***> -- Reply to this email directly or view it on GitHub: #35 (comment) You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

[nvdaes]

For now on my fork PRs are automatically created when sending an add-on and they can be merged, though I don't know why the issue number is not included to close issues automatically. I may continue this the next weekend. If you are interested in this approach let me if tests should be included to ensure that json files conform to the manifest and if removals should be addressed. Aslo, what happens if someone wants to post an add-on from another GitHub account, maybe cc to the corresponding person. Here's an automatic pull request created by GitHub Actions nvdaes#14 I used this action. The author has another action related to automerge: https://github.com/peter-evans/create-pull-request 2022-01-29 23:32 GMT+01:00, Noelia Ruiz Martínez ***@***.***>:

…

Hello, finally I have created an issue form on my addon-datastore fork for testing. I've checked that sending the URL and source url, and checking the stable checkbox, eMule add-on is downloaded automatically, the json file is generated and sent to master for now. When the file exists, a ValueError exception is raised to prevent overriding. I think it's better to create PRs instead of sending add-ons diretly, and work needs to be done to request removals, and also to close issues when pull requests are merged: https://github.com/nvdaes/addon-datastore/issues/new/choose 2022-01-27 7:40 GMT+01:00, Noelia Ruiz Martínez ***@***.***>: > For reference, I'll try to use this package: > > https://pythonrepo.com/repo/ghandic-jsf-python-josn > > 2022-01-27 4:08 GMT+01:00, Noelia Ruiz Martínez ***@***.***>: >> Hello, thanks for your valuable work. I"m experimenting basing a >> workflow >> on >> a GitHub workflow created by you on the transform repo. I"m trying to >> download the add-on from an issue form, providing the url, and then >> generating the json file from the manifest, with >> getManifest(addonDestPath) >> method, checking out the add-on-store-validation repo. I"m thinking >> about >> creating a separate py file with getManifest, similar to sha256.py. >> Your >> idea is good, though the problem is how to decide the trusted >> accounts. > >> > >> Enviado desde mi iPhone > >> > >>> El 27 ene 2022, a las 4:00, Sean Budd ***@***.***> escribió: > >>> > >>>  > >>> Perhaps we could maintain a list of publishers (GitHub accounts) with >>> auto-approval, and other PRs require manual approval. > >>> > >>> — > >>> Reply to this email directly, view it on GitHub, or unsubscribe. > >>> You are receiving this because you authored the thread. > >> > >> > >> -- > >> Reply to this email directly or view it on GitHub: > >> #35 (comment) > >> You are receiving this because you are subscribed to this thread. > >> > >> Message ID: ***@***.***> > > > -- > Reply to this email directly or view it on GitHub: > #35 (comment) > You are receiving this because you are subscribed to this thread. > > Message ID: ***@***.***> -- Reply to this email directly or view it on GitHub: #35 (comment) You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

[feerrenrut]

I'm not a huge fan of "authorized accounts", it's too easy for it to become a gatekeeping mechanism.

I think it is enough to check that existing add-on metadata can't change publisher (ie GitHub owner / org) without manual review. This leaves a race condition for new submissions: EG

• friendlyDev/myAddon is created but not yet submitted
• evilDev/myAddon forks, modifies, and submits

In these cases, I'd suggest we start by relying on the community to inspect the source repo's and flag any problems.
In the future we can consider introducing more ways of verifying add-ons.

[nvdaes]

I think we should update GitHub script action to v5 from v4, used here currently. Then, if this needs to be manually reviewed, not rejected, we may fetch the diff url as suggested in the readme of this action and, if + "publisher" is included, a comment maybe created.This accepting PRs from the json file created automatically from the issue. I think this is not too difficult to achieve. I'll try this the next weekend or maybe before since it's not so difficult, imo. 2022-01-31 3:40 GMT+01:00, Reef Turner ***@***.***>:

…

I'm not a huge fan of "authorized accounts", it's too easy for it to become a gatekeeping mechanism. I think it is enough to check that existing add-on metadata can't change publisher (ie GitHub owner / org) without manual review. This leaves a race condition for new submissions: EG - friendlyDev/myAddon is created but not yet submitted - evilDev/myAddon forks, modifies, and submits In these cases, I'd suggest we start by relying on the community to inspect the source repo's and flag any problems. In the future we can consider introducing more ways of verifying add-ons. -- Reply to this email directly or view it on GitHub: #35 (comment) You are receiving this because you authored the thread. Message ID: ***@***.***>

[nvdaes]

Hello: I've eabled automerge on my fork of addon-datastore and have created a PR from issue 10 sending eMule, automerged after validation. I'll add a step to check diff url to ensure that - "publisher" is not present. This provide some security about the fact that an add-on version is not modified after publication. Of course someone can publish an add-on with an existing id and other version, but this maybe desirable in case previous publishers cannot maintain add-ons. But at least we can be quite sure that a published version won't be mangled like I did here for testing. My intention is to send a PR here and against addon-datastore-validation to check diffs. Note that automerge is enabled when the PR is send from an issue, which requires set an issue template: PR automerged on my fork is at nvdaes#26 2022-01-31 6:54 GMT+01:00, Noelia Ruiz Martínez ***@***.***>:

…

I think we should update GitHub script action to v5 from v4, used here currently. Then, if this needs to be manually reviewed, not rejected, we may fetch the diff url as suggested in the readme of this action and, if + "publisher" is included, a comment maybe created.This accepting PRs from the json file created automatically from the issue. I think this is not too difficult to achieve. I'll try this the next weekend or maybe before since it's not so difficult, imo. 2022-01-31 3:40 GMT+01:00, Reef Turner ***@***.***>: > I'm not a huge fan of "authorized accounts", it's too easy for it to > become > a gatekeeping mechanism. > > I think it is enough to check that existing add-on metadata can't change > publisher (ie GitHub owner / org) without manual review. This leaves a > race > condition for new submissions: EG > - friendlyDev/myAddon is created but not yet submitted > - evilDev/myAddon forks, modifies, and submits > > In these cases, I'd suggest we start by relying on the community to > inspect > the source repo's and flag any problems. > In the future we can consider introducing more ways of verifying add-ons. > > -- > Reply to this email directly or view it on GitHub: > #35 (comment) > You are receiving this because you authored the thread. > > Message ID: ***@***.***> -- Reply to this email directly or view it on GitHub: #35 (comment) You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

[nvdaes]

Hello. I'can show tests with two add-ons: eMule 5.1, published by me (nvdaes), and readFeeds 13.0, whose publisher has been set in the json file to test: CheckMetadata pass for the first one: https://github.com/nvdaes/addon-datastore/runs/5058183875 I have performed gith push and the PR has been merged into master since the json file is downloaded. The second one is readFeeds 13.0. As expected using diff url, GitHub Script action throws an error: "Publishers don't match: So the PR is not merged. My intention is to submit PRs here and in the addon-store-validation repos this weekend. https://github.com/nvdaes/addon-datastore/runs/5058126815 2022-02-02 7:45 GMT+01:00, Noelia Ruiz Martínez ***@***.***>:

…

Hello: I've eabled automerge on my fork of addon-datastore and have created a PR from issue 10 sending eMule, automerged after validation. I'll add a step to check diff url to ensure that - "publisher" is not present. This provide some security about the fact that an add-on version is not modified after publication. Of course someone can publish an add-on with an existing id and other version, but this maybe desirable in case previous publishers cannot maintain add-ons. But at least we can be quite sure that a published version won't be mangled like I did here for testing. My intention is to send a PR here and against addon-datastore-validation to check diffs. Note that automerge is enabled when the PR is send from an issue, which requires set an issue template: PR automerged on my fork is at nvdaes#26 2022-01-31 6:54 GMT+01:00, Noelia Ruiz Martínez ***@***.***>: > I think we should update GitHub script action to v5 from v4, used here > currently. Then, if this needs to be manually reviewed, not rejected, > we may fetch the diff url as suggested in the readme of this action > and, if + "publisher" is included, a comment maybe created.This > accepting PRs from the json file created automatically from the issue. > I think this is not too difficult to achieve. > I'll try this the next weekend or maybe before since it's not so > difficult, > imo. > > 2022-01-31 3:40 GMT+01:00, Reef Turner ***@***.***>: >> I'm not a huge fan of "authorized accounts", it's too easy for it to >> become >> a gatekeeping mechanism. >> >> I think it is enough to check that existing add-on metadata can't >> change >> publisher (ie GitHub owner / org) without manual review. This leaves a >> race >> condition for new submissions: EG >> - friendlyDev/myAddon is created but not yet submitted >> - evilDev/myAddon forks, modifies, and submits >> >> In these cases, I'd suggest we start by relying on the community to >> inspect >> the source repo's and flag any problems. >> In the future we can consider introducing more ways of verifying >> add-ons. >> >> -- >> Reply to this email directly or view it on GitHub: >> #35 (comment) >> You are receiving this because you authored the thread. >> >> Message ID: ***@***.***> > > > -- > Reply to this email directly or view it on GitHub: > #35 (comment) > You are receiving this because you are subscribed to this thread. > > Message ID: ***@***.***> -- Reply to this email directly or view it on GitHub: #35 (comment) You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

[nvdaes]

I've created PR #37
and PR 8 in addon-store-validation repo, with unit tests. It seems to work for me.

[feerrenrut]

I'm going to close this PR, I believe the problem is captured with #41