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

"Branch already exists" during scan-repository fix #649

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

eranturgeman
Copy link
Contributor

@eranturgeman eranturgeman commented Feb 26, 2024

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • This pull request is on the dev branch.
  • I used gofmt for formatting the code before submitting the pull request.

This PR addresses a bug in the scan-repository flow. When multiple descriptor files contain at least one similar fix (package name + version), Frogbot encounters an issue when attempting to fix the second descriptor due to duplication in the 'fix' branch names. To resolve this issue, we have introduced a new field in the frogbot-config.yml file under the 'projects' section called 'projectName'. By adding this field to an entry in the 'projects' array, it enables differentiation between two similar fixes, resulting in unique branch names and titles (the unique identifier is appended to the title).

SCENARIOS:

  • You receive an error indicating "a branch named XXX already exists."
  • You have multiple descriptors to scan, and among them there are some with similar vulnerabilities that need fixing.
  • You manually specified working directories for scanning and encounter duplicate vulnerabilities among them.
  • You manually divided your scanned repository into projects, resulting in duplicate vulnerabilities.

ACTION PLAN:

  1. Organize your scanned repository into distinct 'projects' by creating separate entries under the 'projects' array in frogbot-config.yml (an example for splitting descriptors for a python project was added below)
  2. Ensure there are no duplicated packages that needs to be fixed within the same project.
  3. Assign a unique key to each project using the 'projectName' field.
  4. Set 'aggregateFixes' to FALSE.
  5. Commit and push the updated frogbot-config.yml to your target branch.
  6. Re-run the scan.

Screenshot 2024-03-05 at 10 44 36

IMPORTANT:
As of its current architecture, Frogbot does NOT offer support for multiple descriptor files.
This fix serves as a temporary "workaround" as achieving full support for this feature would necessitate significant architectural adjustments.

CAUTION:
If you opt to utilize this new feature and you already have existing PRs from Frogbot, please note that new PRs will be generated without removing the old ones!

@eranturgeman eranturgeman added bug Something isn't working safe to test Approve running integration tests on a pull request labels Feb 28, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Feb 28, 2024
@eranturgeman eranturgeman changed the title 'Branch already exists' fix "Branch already exists" during scan-repository fix Feb 28, 2024
@eranturgeman eranturgeman added the safe to test Approve running integration tests on a pull request label Feb 28, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Feb 28, 2024
@eranturgeman eranturgeman marked this pull request as ready for review February 29, 2024 09:26
@eranturgeman eranturgeman added the safe to test Approve running integration tests on a pull request label Feb 29, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Feb 29, 2024
Copy link
Contributor

👍 Frogbot scanned this pull request and found that it did not add vulnerable dependencies.


Comment on lines 498 to 506
/*
handler := cfp.handlers[vulnDetails.Technology]
if handler == nil {
handler = packagehandlers.GetCompatiblePackageHandler(vulnDetails, cfp.scanDetails)
cfp.handlers[vulnDetails.Technology] = handler
} else if _, unsupported := handler.(*packagehandlers.UnsupportedPackageHandler); unsupported {
return
}
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

?

utils/params.go Outdated Show resolved Hide resolved
Copy link
Contributor

@attiasas attiasas left a comment

Choose a reason for hiding this comment

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

I suggest when parsing the params, if more than one project is defined and no identify exists, to set the names with a counter (or default value like 'project-1', 'project-2'... This will fix the issues for users that did not changed their config with special names

schema/frogbot-schema.json Outdated Show resolved Hide resolved
schema/frogbot-schema.json Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants