Skip to content

Use Files::MultiService to generate patch for Duo Vulnerability Resolution

Michael Beckerrequested to merge
497193-FF-replace-patch-generation-with-file-rewriting into master

What does this MR do and why?

Context

We have quite a few customer contacts about the Duo Vulnerability feature failing to create an MR. We also have a bunch of these errors in the logs that appear to be related.

After an investigation1 into the root cause, we realized that there are actually two bugs occurring:

  1. this line, which is used to gsub the llm fix into the source file, is too brittle.
    • We have examples in the logs where we would expect a patch to be created, but instead the gsub fails to update the source code (because it fails to detect the old code)
  2. The hand-rolled patch creation code makes an invalid git patch in some situations, causing Commits::CommitPatchService to raise

This Change

This changes addresses 2 from above.

Rather than rolling a valid git patch by hand, to be used by the Commits::CommitPatchService, I am instead passing the patched source file to Files::MultiService. This let's us just update the entire file contents in a branch. The diff then is provide by git by opening an MR to the default branch to modify the source file directly. 2

logic before logic after

The previous code flow was to:

  1. extract the fixes from the llm response
  2. make a copy of the source code containing the vulnerability
  3. call .gsub on the copied source code with the fixes from 13
    • we now have original_source_code and patched_source_code
  4. hand-roll a git diff and patch using original_source_code and patched_source_code 4
  5. Send that hand rolled patch into Commits::CommitPatchService to create an branch.
  6. create the MR with the created branch

This MR removes step 4, instead doing:

  1. extract the fixes from the llm response
  2. make a copy of the source code containing the vulnerability
  3. call .gsub on the copied source code with the fixes from 13
    • we now have original_source_code and patched_source_code
  4. send patched_source_code to Files::MultiService, telling it to make this the new contents for that file
    • this is done on a separate branch
    • all the hand rolled diff logic goes away, as it now happens by just diffing the branches
  5. create the MR with the created branch

Screenshots or screen recordings

before after
Screencast_from_2025-05-26_18-21-12 Screencast_from_2025-05-26_18-55-18

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

epic: &17227
related to: #536007
related to: #497193 (closed)
related to: #526882
Changelog: fixed
EE: true

  1. The investigation started in this issue and continued in this issue, where the fix was determined ↩

  2. Files::MultiService is the service backing this API ↩

  3. This is a source of errors. The gsub is failing to replace `` with new_code. This MR does not touch this code as changing this behavior is considered too risky. I hope this series of MRs reduces the affected surface area of this bug, making it no longer considered risky. ↩ ↩2

  4. This is an additional source of errors. These errors will be resolved in this MR. ↩

Edited by Michael Becker

Merge request reports