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

Include new app_blocking_match_based_ids field in ci_scan_complete_response #326

Merged
merged 4 commits into from
Dec 6, 2024

Conversation

mmcqd
Copy link
Member

@mmcqd mmcqd commented Dec 5, 2024

Right now whether or not a CI scan should exit with code 1 is determined mostly by the CLI, but the app can use the app_block_override field of ci_scan_complete_response to tell the CLI to exit with code 1 no matter what. This is currently used to support blocking scans when we detect a dependency with a blocked license.

We're currently working on a project which includes making it more customizable within the app whether or not a given finding should be blocking, right now just for supply chain, but with the aim to eventually move the computation of blocking findings entirely to the app. So we'd like to be able to tell the CLI not only "please block" but also "here are the list of findings that the app determined are blocking".

  • I ran make setup && make to update the generated code after editing a .atd file (TODO: have a CI check)
  • I made sure we're still backward compatible with old versions of the CLI.
    For example, the Semgrep backend need to still be able to consume data
    generated by Semgrep 1.50.0.
    See https://atd.readthedocs.io/en/latest/atdgen-tutorial.html#smooth-protocol-upgrades
    Note that the types related to the semgrep-core JSON output or the
    semgrep-core RPC do not need to be backward compatible!

@mmcqd mmcqd requested a review from pangngan December 5, 2024 21:25
Copy link

github-actions bot commented Dec 5, 2024

Backwards compatibility summary:

Checking backward compatibility of semgrep_output_v1.atd against past version v1.49.0
Skipping v1.50.0 because commit 857682f41eb09e0b330a247ff1adf3bfeaf9d9ca has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.52.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.53.0
Skipping v1.54.0 because commit 3b72d494260258497e796d094b1a4916501a6df1 has already been checked
Skipping v1.54.1 because commit 3b72d494260258497e796d094b1a4916501a6df1 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.54.2
Skipping v1.54.3 because commit 9f1c50383a9a9969e2fe7a5f9bff9ca0a7c837bb has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.55.0
Skipping v1.55.1 because commit 6dffeaa692153fd33b4f154fddaefde1f2f1ae27 has already been checked
Skipping v1.55.2 because commit 6dffeaa692153fd33b4f154fddaefde1f2f1ae27 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.56.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.57.0
Skipping v1.58.0 because commit 4cc11b00d411c02fc611aa8c78a336520438fb48 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.59.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.59.1
Checking backward compatibility of semgrep_output_v1.atd against past version v1.60.0
Skipping v1.60.1 because commit eed58a091fd7d19e402a6d4cf2d287e137215d03 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.61.0
Skipping v1.61.1 because commit bbfd1c5b91bd411bceffc3de73f5f0b37f04433d has already been checked
Skipping v1.62.0 because commit bbfd1c5b91bd411bceffc3de73f5f0b37f04433d has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.63.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.64.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.65.0
Skipping v1.66.0 because commit 3e7bbafa2b7e722d893303a7fb90a83dab6737a7 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.66.1
Skipping v1.66.2 because commit 215a54782174de84f97188632b4a37e35ba0f827 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.67.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.68.0
Skipping v1.69.0 because commit d5b91fa4f6a03240db31e9bbbc5376a99bc8eeea has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.70.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.71.0
Skipping v1.72.0 because commit 75abf193687b84ab341d8267d865ad68d81a89c9 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.73.0
Skipping v1.74.0 because commit 9f38254957c50c68ea402eebae0f7aa40dd01cbf has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.75.0
Skipping v1.76.0 because commit 9102031608aa4154e1c37f557550ec4eabc8780c has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.77.0
Skipping v1.78.0 because commit dcb5d77b420ddee61f58aadd3c2c7aef38778154 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.79.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.80.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.81.0
Skipping v1.82.0 because commit 9e0f3bec26b07b4fb6753a32cb75277f45f2572c has already been checked
Skipping v1.83.0 because commit 9e0f3bec26b07b4fb6753a32cb75277f45f2572c has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.84.0
Skipping v1.84.1 because commit 3daef49297ada205359cc1d2996354c94b628b0d has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.85.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.86.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.87.0
Skipping v1.88.0 because commit 512c0bd97db59c48a5705b2741662a338776e438 has already been checked
Skipping v1.89.0 because commit 512c0bd97db59c48a5705b2741662a338776e438 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.90.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.91.0
Skipping v1.92.0 because commit 2351c5e528cb7430422208dc66707894c066b508 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.93.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.94.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.95.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.96.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.97.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.98.0
Skipping v1.99.0 because commit 60809032a2e39742f42910d46b3e5dd305b8b8cf has already been checked

@aryx aryx requested a review from chmccreery December 5, 2024 22:01
semgrep_output_v1.atd Outdated Show resolved Hide resolved
@chmccreery
Copy link
Contributor

chmccreery commented Dec 5, 2024

Sorry, going to leave some high-level feedback here since I don't have context on the project. Feel free to ping me in Slack or put time on my calendar if you want to discuss in person:

How would we know ahead of time which match_based_id's are blocking? We don't know a match_based_id for a particular finding until we've seen that finding at least once - and we don't know the various qualities of that finding until it's been detected at least once, so don't know whether or not to block on it. Seems like a chicken and egg problem, but maybe I'm missing something?

EDIT: I now realize this is part of the complete response, not create, so it doesn't necessarily have the problem above, but it moves us further away from the async results processing that we have been working towards. I'm not sure of a solution here right now, but it might be worth brainstorming together if there's a way to separate this "calculating blocking or not" piece from the rest of "process all findings" so we aren't forever saying a scan can't complete until all of its results have finished uploading.

@mmcqd
Copy link
Member Author

mmcqd commented Dec 5, 2024

Discussed this with @chmccreery on a call, we got the go ahead

@mmcqd mmcqd merged commit 37314fd into main Dec 6, 2024
3 checks passed
@mmcqd mmcqd deleted the matthew/app-actions-data branch December 6, 2024 23:55
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.

3 participants