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

feat(scoop-virustotal): Migrate to VirusTotal API v3 #4983

Merged
merged 30 commits into from
Jun 15, 2022

Conversation

ClassicDarkChocolate
Copy link
Contributor

@ClassicDarkChocolate ClassicDarkChocolate commented Jun 10, 2022

…and use standard psstreams

Description

  • The command now outputs VirusTotal report as a PowerShell object to Success stream and other contents to Information/Warning stream.
  • Migrated to VirusTotal API v3. The command now requires a VirusTotal API key to use.
  • The new search flow:
    1. Search for file report by file hash.
      • If it fails, go to step 2.
      • If file report is found, shows it. Go to step 4.
    2. Search for url report by file url.
      • If no url report is found, go to step 3.
      • If url report is found, and the report shows the file hash. Search for file report and shows it. Go to step 4.
      • If url report is found, and the report does not have file hash. Tell user to manually upload file. Go to step 4.
    3. If --scan is specified, submit url to VirusTotal.
    4. End.
  • Remove Submit-RedirectedUrl because VirusTotal automatically finds the final url.

Motivation and Context

Closes #4965

How Has This Been Tested?

Scans hundreds of apps.

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

@ClassicDarkChocolate
Copy link
Contributor Author

The code is already working. Any suggestions?

@ClassicDarkChocolate ClassicDarkChocolate marked this pull request as ready for review June 11, 2022 10:55
@niheaven
Copy link
Member

Thanks for your work!

Since you've nearly rewrote the whole file, reviewing may take more time, sorry~

Some suggestions, could you turn back to warn, info and error when writing meaningful console output? These functions are Scoop's inner functions and are used for a consistent UX.

@niheaven
Copy link
Member

Is there some method that VT could be used without API Key?

And could you add a hint that could guide users to sign in VT and get the API Key?

@ClassicDarkChocolate
Copy link
Contributor Author

Is there some method that VT could be used without API Key?

The old api used by scoop is blocked:
GET https://www.virustotal.com/ui/files/af17a2803c5c6406b9b60dfef2d34f72f218975f9d78df21005a44f6e2f0caf9

{
  "error": {
    "message": "Please re-send request with a valid reCAPTCHA response in the \"x-recaptcha-response\" header",
    "code": "RecaptchaRequiredError"
  }
}

And could you add a hint that could guide users to sign in VT and get the API Key?

Users only have to register an account. No other actions needed.

@ClassicDarkChocolate
Copy link
Contributor Author

ClassicDarkChocolate commented Jun 14, 2022

The above commit is mainly to fix these issues.

  • When hash of file is incorrect, tell user hash is not matched. Only manifest with sha256 hash can be checked.
    • Can be tested now with scoop download rustdesk; scoop virustotal -sp rustdesk
  • Rarely when file is downloaded by virustotal and hash is returned in url report, but file is still not being scanned somehow.
    • Can be tested with scoop virustotal -sp telnet

I think I won't do another commit after this one. Unless there are bugs or suggestions.

Copy link
Member

@niheaven niheaven left a comment

Choose a reason for hiding this comment

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

Fix help message and add API Key hint

Please add a changelog entry.

libexec/scoop-virustotal.ps1 Outdated Show resolved Hide resolved
libexec/scoop-virustotal.ps1 Outdated Show resolved Hide resolved
ClassicDarkChocolate and others added 5 commits June 15, 2022 00:05
Co-authored-by: Hsiao-nan Cheung <niheaven@gmail.com>
Co-authored-by: Hsiao-nan Cheung <niheaven@gmail.com>
@niheaven niheaven changed the title feat(scoop-virustotal): migrate to virustotal api v3, improve flows, … feat(scoop-virustotal): Migrate to VirusTotal API v3 Jun 15, 2022
@niheaven niheaven merged commit 9e6c758 into ScoopInstaller:develop Jun 15, 2022
@ClassicDarkChocolate ClassicDarkChocolate deleted the virustotal branch June 20, 2022 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants