Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add plugin: BibDesk Integration #4569

Merged
merged 6 commits into from
Feb 12, 2025
Merged

Add plugin: BibDesk Integration #4569

merged 6 commits into from
Feb 12, 2025

Conversation

alberti42
Copy link
Contributor

@alberti42 alberti42 commented Nov 1, 2024

I am submitting a new Community Plugin

Repo URL

Link to my plugin: https://github.com/alberti42/obsidian-bibdesk-integration

Release Checklist

  • I have tested the plugin on
    • Windows
    • macOS
    • Linux
  • My GitHub release contains all required files (as individual files, not just in the source.zip / source.tar.gz)
    • main.js
    • manifest.json
    • styles.css (optional)
  • GitHub release name matches the exact version number specified in my manifest.json (Note: Use the exact version number, don't include a prefix v)
  • The id in my manifest.json matches the id in the community-plugins.json file.
  • My README.md describes the plugin's purpose and provides clear usage instructions.
  • I have read the developer policies at https://docs.obsidian.md/Developer+policies, and have assessed my plugins's adherence to these policies.
  • I have read the tips in https://docs.obsidian.md/Plugins/Releasing/Plugin+guidelines and have self-reviewed my plugin to avoid these common pitfalls.
  • I have added a license in the LICENSE file.
  • My project respects and is compatible with the original license of any code from other plugins that I'm using.
    I have given proper attribution to these other projects in my README.md.

Copy link

github-actions bot commented Nov 1, 2024

Hello!

I found the following issues in your plugin submission

Errors:

❌ Plugin name mismatch, the name in this PR (BibDesk Integration) is not the same as the one in your repo (BibDesk integration). If you just changed your plugin name, remember to change it in the manifest.json in your repo and your latest GitHub release.


This check was done automatically. Do NOT open a new PR for re-validation. Instead, to trigger this check again, make a change to your PR and wait a few minutes, or close and re-open it.

@github-actions github-actions bot changed the title Add BibDesk Integration plugin Add plugin: BibDesk Integration Nov 1, 2024
@ObsidianReviewBot
Copy link
Collaborator

Thank you for your submission, an automated scan of your plugin code's revealed the following issues:

Required

[1]:Using innerHTML, outerHTML or similar API's is a security risk. Instead, use the DOM API or the Obsidian helper functions: https://docs.obsidian.md/Plugins/User+interface/HTML+elements

[1][2][3][4][5][6][7][8][9]:You should avoid assigning styles via JavaScript or in HTML and instead move all these styles into CSS so that they are more easily adaptable by themes and snippets.


Do NOT open a new PR for re-validation.
Once you have pushed all of the required changes to your repo, the bot will update the labels on this PR within 6 hours.
If you think some of the required changes are incorrect, please comment with /skip and the reason why you think the results are incorrect.

@ObsidianReviewBot ObsidianReviewBot self-assigned this Nov 2, 2024
@ObsidianReviewBot ObsidianReviewBot added Changes requested Additional review required PR needs to be reviewed by another person, after the currently requested changes have been made Ready for review and removed Ready for review Changes requested Additional review required PR needs to be reviewed by another person, after the currently requested changes have been made labels Nov 2, 2024
@ObsidianReviewBot ObsidianReviewBot removed their assignment Nov 2, 2024
@ObsidianReviewBot
Copy link
Collaborator

Changes requested by bot have been made, assigning human for additional review.

@joethei
Copy link
Collaborator

joethei commented Nov 7, 2024

Your release main.js contains multiple long strings that are base64 encoded, which contain code, That is obfuscation, and is not allowed as per our developer policies.

constructor(app:App,manifest:PluginManifest) {
This code should be in onload, not in the constructor.

// Path to plugins folder
To get the folder of your plugin use this.manifest.dir

atob("QGFydGljbGV7RWluc3RlaW46MTkzNSwKICAgIGF1dGhvciA9IHtFaW5zdGVpbiwgQS4gYW5kIFBvZG9sc2t5LCBCLiBhbmQgUm9zZW4sIE4ufSwKICAgIGRvaSA9IHsxMC4xMTAzL1BoeXNSZXYuNDcuNzc3fSwKICAgIGpvdXJuYWwgPSB7UGh5cy4gUmV2Ln0sCiAgICBtb250aCA9IHttYXl9LAogICAgbnVtYmVyID0gezEwfSwKICAgIHBhZ2VzID0gezc3N30sCiAgICB0aXRsZSA9IHt7Q2FuIFF1YW50dW0tTWVjaGFuaWNhbCBEZXNjcmlwdGlvbiBvZiBQaHlzaWNhbCBSZWFsaXR5IEJlIENvbnNpZGVyZWQgQ29tcGxldGU/fX0sCiAgICB2b2x1bWUgPSB7NDd9LAogICAgeWVhciA9IHsxOTM1fX0KCkBhcnRpY2xle1dhdHNvbjoxOTUzLAogICAgQXV0aG9yID0ge1dhdHNvbiwgSi4gRC4gYW5kIENyaWNrLCBGLiBILiBDLn0sCiAgICBUaXRsZSA9IHt7TW9sZWN1bGFyIFN0cnVjdHVyZSBvZiBOdWNsZWljIEFjaWRzOiBBIFN0cnVjdHVyZSBmb3IgRGVveHlyaWJvc2UgTnVjbGVpYyBBY2lkfX0sCiAgICBKb3VybmFsID0ge05hdHVyZX0sCiAgICBZZWFyID0gezE5NTN9LAogICAgVm9sdW1lID0gezE3MX0sCiAgICBOdW1iZXIgPSB7NDM1Nn0sCiAgICBQYWdlcyA9IHs3Mzd9LAogICAgTW9udGggPSB7YXByfSwKICAgIERvaSA9IHsxMC4xMDM4LzE3MTczN2EwfSwKfQoKQGFydGljbGV7RnJhbmtsaW46MTk1MywKICAgIEF1dGhvciA9IHtGcmFua2xpbiwgUm9zYWxpbmQgRS4gYW5kIEdvc2xpbmcsIFIuIEcufSwKICAgIFRpdGxlID0ge3tFdmlkZW5jZSBmb3IgMi1DaGFpbiBIZWxpeCBpbiBDcnlzdGFsbGluZSBTdHJ1Y3R1cmUgb2YgU29kaXVtIERlb3h5cmlib251Y2xlYXRlfX0sCiAgICBKb3VybmFsID0ge05hdHVyZX0sCiAgICBZZWFyID0gezE5NTN9LAogICAgVm9sdW1lID0gezE3Mn0sCiAgICBOdW1iZXIgPSB7NDM2OX0sCiAgICBQYWdlcyA9IHsxNTZ9LAogICAgTW9udGggPSB7anVsfSwKICAgIERvaSA9IHsxMC4xMDM4LzE3MjE1NmEwfSwKfQo=")
Just include the actual content in the plugin as a string, having this base64 encoded looks very phishy.

function max(a:number,b:number): number {
Why are you reimplementing these instead of using Math.max and Math,min ?

@joethei joethei added Changes requested Installation not recommended Submission does not conform to developer policies, or includes potentially dangerous code. Minor changes requested PR can be merged after some final changes have been requested and removed Ready for review labels Nov 7, 2024
alberti42 added a commit to alberti42/obsidian-bibdesk-integration that referenced this pull request Nov 8, 2024
@alberti42
Copy link
Contributor Author

Thanks! I did all changes.

PS: Do you get notificiations when I post a message like this, or I need to tag you to inform you when changes are made?

Copy link
Contributor Author

@alberti42 alberti42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made requested changes to bibdesk-integration. New version 1.0.2 available.

@ObsidianReviewBot ObsidianReviewBot added Ready for review Changes made and removed Changes requested Minor changes requested PR can be merged after some final changes have been requested labels Nov 8, 2024
@alberti42
Copy link
Contributor Author

Dear Jothei, what does the flag "Installation not recommended" mean? Is the plugin rejected or some changes are needed?

@alberti42
Copy link
Contributor Author

Ok. I think I understood why the flag Installation not recommended was added. Because of the obfuscated code which you pointed out above, right? OK. In the resubmission of the plugin, I avoided any base64 encoding, just pure text.

@joethei
Copy link
Collaborator

joethei commented Jan 2, 2025

Sorry for the delay.

# Obsidian BibDesk Integration
Naming something "Obsidian xyz" is reserved for first party products we create, please change this header.

"description": "Allows using BibTex citations and opening attached PDFs from a BibDesk library.",
Try to avoid starting your description with "Allows you to" or similar wording.
A lot of plugin descriptions already use that, it's getting a bit repetitive.
We have published a guide for plugin descriptions to follow here: https://docs.obsidian.md/Plugins/Releasing/Submission+requirements+for+plugins#Keep+plugin+descriptions+short+and+simple.

@joethei joethei added Changes requested Minor changes requested PR can be merged after some final changes have been requested and removed Ready for review Installation not recommended Submission does not conform to developer policies, or includes potentially dangerous code. Changes made labels Jan 2, 2025
@alberti42
Copy link
Contributor Author

Thanks!

Yes, I changed the header in the README.md and updated the description. The new changes are included in release 1.0.3

Copy link

github-actions bot commented Feb 2, 2025

Hi there, as this PR has not seen any activity in the last 30 days, it will be closed in 15 days unless there are any updates.

@github-actions github-actions bot added stale and removed stale labels Feb 2, 2025
@alberti42
Copy link
Contributor Author

alberti42 commented Feb 2, 2025

@joethei I know you have a lot of work and probably some backlog. Is there anything more has to be done on this plugin? It was nicely crafted, and I am sure it will be helpful for people to manage their scientific library. It is many months since it was submitted. Thanks for your consideration.

@alberti42
Copy link
Contributor Author

@joethei Is there anything I should do on my side or simply wait? Perhaps you are simply busy with other projects? A short answer would be greatly appreciated.

@joethei
Copy link
Collaborator

joethei commented Feb 12, 2025

You have not updated the description in the manifest.

alberti42 added a commit to alberti42/obsidian-bibdesk-integration that referenced this pull request Feb 12, 2025
@alberti42
Copy link
Contributor Author

@joethei Thanks! I had changed package.json but forgot to also adjust manifest.json with the new description.

Now the new release 1.0.4 includes the new description.

@ObsidianReviewBot ObsidianReviewBot added Ready for review Changes made and removed Changes requested Minor changes requested PR can be merged after some final changes have been requested labels Feb 12, 2025
@joethei joethei merged commit 4aa1955 into obsidianmd:master Feb 12, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants