search

facebook / pysa-action

GitHub Action for Pysa
MIT License
18 stars 11 forks source link

Fixes #1: Add option to build with poetry #2

Closed abishekvashok closed 1 year ago

abishekvashok commented 1 year ago

Add option to install dependencies with poetry in the action. Signed-off-by: Abishek V Ashok abishekvashok@gmail.com

Poetry docs: https://python-poetry.org/docs/#installing-with-pipx https://python-poetry.org/docs/basic-usage/#installing-dependencies-only

Why --no-root: poetry installs projects as editable by default with --no-root we get the same behavior as pip install.

Fixes #1

arthaud commented 1 year ago

It looks like there are CI errors, could you look into those?

abishekvashok commented 1 year ago

@arthaud I think it is fixed now. Sorry for messing up earlier. You might want to re-approve for tests.

Also consider, removing needing of approval for workflows - I don't think we are anywhere near for Github actions in this repo :)

arthaud commented 1 year ago

Well, it looks like there are failures on master now, could you look? https://github.com/facebook/pysa-action/actions/runs/4534111680/jobs/7987854828

abishekvashok commented 1 year ago

@arthaud sure :) It seems unrelated to this changes.

Do we have past logs stored elsewhere? The past github action logs have expired.

I see the primary cause being pysa unable to detect issues as we aren't including the modules in the search path. The action that fails detects 0 issues while the one succeeding has found 8. Thus sapp creates an empty json file which is not happily accepted by the github sariff parser. The only change between the actions is that one uses .pyre_configuration and the other doesn't. I believe adding LD_LIBRARY_PATH to search_path will fix this (make pysa able to find the issues.) I will also try to make a pr for sapp to atleast output an empty file confronting to the sariff specs if there are no vulnerabilities :)

Also for the logs, consider enabling monthly run of the github workflows in this repository. The expiration time for logs seem to be 90 days (https://docs.github.com/en/actions/using-workflows/storing-workflow-data-as-artifacts), so we can have at least 2 logs for each workflow available at any time.

abishekvashok commented 1 year ago

@arthaud

So took a bit of time to dig into this. Including every module in the path was not the solution but that the submodule is too old (snapshot is from a commit in 2022, pysa and pyre-check have been both changed a lot since then)

I made a PR to fix this and stop relying on submodules/snapshots: https://github.com/facebook/pysa-action/pull/3 Also, for SAPP, to conform to SARIF: https://github.com/facebook/sapp/pull/92