search

10up / wpcli-vulnerability-scanner

WP-CLI command for checking installed plugins and themes for vulnerabilities reported on wpvulndb.com
MIT License
274 stars 40 forks source link

Add Vulnerability Reference link information. #84

Closed iamdharmesh closed 1 year ago

iamdharmesh commented 1 year ago

Is your enhancement related to a problem? Please describe.

Add Vulnerability Reference link information to provide the functionality to users to view more details on vulnerability. the default behavior should be like what we have currently, reference link information should be added only when the --reference flag gets passed to the command.

Designs

No response

Describe alternatives you've considered

No response

Code of Conduct

iamdharmesh commented 1 year ago

Update: Started work on this, blocked by the reference link break issue in table format. Click on the link redirects to the partial link. Checking on a solution.

image

jeffpaul commented 1 year ago

I wonder if Wordfence generates a short URL for those? If not, we could look at using a short URL service to do that? Alternatively we could just remove the linking on the first line and let folks copy/paste to view the full URL.

bmarshall511 commented 1 year ago

Hey @jeffpaul and @iamdharmesh! 👋 This isn't exactly my area of expertise, but I'm always up for a challenge! 💪 I dug into the code and did some googling. Based on my limited knowledge and what I found, correct me if I'm wrong, but it seems like you can't define links in the output directly. It's actually up to your terminal to parse them. Taking a look at the code, it seems like it's inline since I couldn't find anything that would differentiate a link from a text URL. I ran the code you provided in the screenshot above, but unfortunately, my iTerm didn't automatically create a link for it—it just displayed it as plain text.

image

Is there a setting I need to enable in order for the links to be displayed properly?

iamdharmesh commented 1 year ago

Hi @bmarshall511 👋 ,

Thanks for the comment and checking on this.

but it seems like you can't define links in the output directly. It's actually up to your terminal to parse them.

Yes, it seems correct that it's up to the terminal to parse them.

I ran the code you provided in the screenshot above, but unfortunately, my iTerm didn't automatically create a link for it—it just displayed it as plain text.

Yes, link are displayed as a plain text and I didn't found anything yet to differentiate it from the text. However, you will be able to click the link by pressing Command + click. (OR CTRL + click).

The bigger blocker I see here is broken links. WP CLI table output prints links in multiple line. So, terminal consider each line as a separate string and ultimately only first part of reference link parsed as link which is broken due to multiple lines.

@Sidsector9 do you have any idea on how to handle this?

cc: @jeffpaul

bmarshall511 commented 1 year ago

I wonder if Wordfence generates a short URL for those? If not, we could look at using a short URL service to do that? Alternatively we could just remove the linking on the first line and let folks copy/paste to view the full URL.

@jeffpaul Haven't found anything about Wordfence generating short URLs. As for the URL shortening service, all I've found require an API key & only one I've ran into seem to be free. Considering that our target users are likely devs who are proficient in copy-pasting, wondering if adding a URL shortening service might be overkill. It would require extra steps like registratio, obtaining & sending an API key just for the --reference flag. What do you think?

I tend to lean toward just letting people copy/paste themselves & if the terminal happens to be large enough where the links don't wrap, they could always click — though would be great if we could find a way to tell the terminal to parse the wrapped text as a single link.

jeffpaul commented 1 year ago

@bmarshall511 I agree, fine to skip the clickability feature (we can always open a separate issue to track that if there's an easy, straightforward way to handle in the future)

iamdharmesh commented 1 year ago

This was discussed in an internal team meeting yesterday, and it was decided to proceed by simply adding a reference column when the user utilizes the --reference flag. This decision is based on the understanding that users always have the option to enlarge the terminal window to prevent the wrapping of reference links. Additionally, implementing this solution will provide benefits in other formats such as CSV and JSON, where the issue of URL wrapping will not be encountered.

@bmarshall511 Please free to pick this up if you have enough bandwidth and this is something that interests you.

Thanks

cc: @jeffpaul

bmarshall511 commented 1 year ago

@iamdharmesh Happy to, but think you've already got the work done for it on the https://github.com/10up/wpcli-vulnerability-scanner/compare/enhancement/84. I was using that branch to test & look into the linking issue. Everything seemed to work as expected when adding the --reference flag. Was there something more that needs to be added?

iamdharmesh commented 1 year ago

I was using that branch to test & look into the linking issue. Everything seemed to work as expected when adding the --reference flag. Was there something more that needs to be added?

Yeah, I had implemented it for Wordfence only and noticed this issue so, stopped further work on this. But yeah as this functionality is implemented in a base class, there is not much to do here, I will take care of it. I really appreciate your help in checking this and sharing findings for potential solutions.

Thanks