cds-snc / notification-planning-core

Project planning for GC Notify Core Team
0 stars 0 forks source link

Investigate AI Review for our PRs #403

Open jimleroyer opened 3 months ago

jimleroyer commented 3 months ago

Description

As a developer, I want to have my code reviewed by AI, So that it can catches common errors that humans might overlook once in a while Such sensitive Terraform variables not being flagged as such in the code changes.

WHY are we building?

This is following an incident action items as we did not catch a sensitive variable and that caused a leak through our logs. We want to catch that sort of bugs as early as possible in the development pipeline. An AI could catch these sort of easy to spot (but potentially easy to miss) changes and alert us on the pull requests.

WHAT are we building?

Integrating with an AI that reviews our pull requests. Namely, this one was spotted and could be tried: https://github.com/marketplace/actions/ai-code-review-action

VALUE created by our solution

More security for this specific use case, but also higher code quality in general by an AI bot reviewing our code.

This might mean some noises too as well on code changes that the AI bit might be incorrect or unaware of context.

Acceptance Criteria

QA Steps

P0NDER0SA commented 3 months ago

We need to get an OpenAI API key

P0NDER0SA commented 3 months ago

https://github.com/cds-snc/notification-api/pull/2254

ben851 commented 2 months ago

@ben851 to QA

ben851 commented 2 months ago

Waiting on Calvin to give us a new API key so we can re-enable the PR bot.

ben851 commented 2 months ago

We received the API key, and merged the new job and trigger. Needs to go back onto terraform

sastels commented 1 month ago

Steve will try on one of his terraform PRs

sastels commented 1 month ago

looked good against my terraform PR! Haven't tried a manifest one yet though.

sastels commented 1 month ago

Steve will make a PR to add a fake password or something that's not marked sensitive.

sastels commented 1 month ago

AI said variable critical_government_password should be marked sensitive and did not say that variable google_url should be sensitive ✅

sastels commented 1 month ago

did a manifest PR but it was just changing a variable - AI review ran but had nothing t0 say. We need to run with a more substantive manifest pr...

P0NDER0SA commented 1 month ago

We might open this up to some other repositories to get more feedback because it's not costing a lot of money so far!

P0NDER0SA commented 2 weeks ago

We are going to run this until Xmas and then review the costs and benefits for this and make a decision!

P0NDER0SA commented 2 weeks ago

going to add a slack reminder for sometime around xmas

P0NDER0SA commented 2 weeks ago

I added a slack reminder for December 20th