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

Add attack track controls to gitRegoStore #496

Merged
merged 4 commits into from
Aug 10, 2023

Conversation

YiscahLevySilas1
Copy link
Collaborator

Overview

This PR adds a field that holds all attackTrack controls in the gitRegoStore object. This is to prevent the attack chain engine from having to calculate this list for over run, which would be very not efficient

Signed-off-by: YiscahLevySilas1 <yiscahls@armosec.io>
@codiumai-pr-agent-free
Copy link
Contributor

PR Analysis

  • 🎯 Main theme: Adding attack track controls to gitRegoStore
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: True
  • Focused PR: Yes, the PR is focused on adding attack track controls to gitRegoStore.
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • General suggestions: The PR is well-structured and follows good practices. The new functionality is properly tested. However, it would be beneficial to add more comments in the code to explain the logic and the purpose of the new methods.

  • 🤖 Code feedback:

    • relevant file: gitregostore/gitstoreutils.go
      suggestion: Consider handling the error returned by the setAttackTracksControls method. Currently, the error is ignored which might lead to unexpected behavior. [important]
      relevant line: gs.setAttackTracksControls()

    • relevant file: gitregostore/gitstoreutils.go
      suggestion: It seems that the setAttackTracksControls method does not need to return an error as it currently never returns one. Consider removing the error return value to simplify the code. [medium]
      relevant line: func (gs *GitRegoStore) setAttackTracksControls() error {

    • relevant file: gitregostore/gitstoreutils.go
      suggestion: It would be more efficient to use a capacity while initializing the allAttackTrackControls slice since the maximum size is known (the length of gs.Controls). [medium]
      relevant line: allAttackTrackControls := []opapolicy.Control{}

How to use

To invoke the PR-Agent, add a comment using one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve: Suggest improvements to the code in the PR.
/ask <QUESTION>: Pose a question about the PR.

To edit any configuration parameter from 'configuration.toml', add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, use the /config command.

@github-actions
Copy link
Contributor

Summary:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: failure
  • Unit test: success
  • Go linting: failure

Signed-off-by: YiscahLevySilas1 <yiscahls@armosec.io>
Signed-off-by: YiscahLevySilas1 <yiscahls@armosec.io>
Signed-off-by: YiscahLevySilas1 <yiscahls@armosec.io>
@github-actions
Copy link
Contributor

Summary:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: failure
  • Unit test: success
  • Go linting: failure

@YiscahLevySilas1 YiscahLevySilas1 merged commit d191ac9 into master Aug 10, 2023
26 checks passed
@YiscahLevySilas1 YiscahLevySilas1 deleted the attack-chain-controls branch September 3, 2023 10:23
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.

2 participants