Skip to content

feat: add Security Analyzer functionality#3058

Merged
neubig merged 95 commits intoOpenHands:mainfrom
invariantlabs-ai:security_analyzer
Aug 13, 2024
Merged

feat: add Security Analyzer functionality#3058
neubig merged 95 commits intoOpenHands:mainfrom
invariantlabs-ai:security_analyzer

Conversation

@adrgs
Copy link
Copy Markdown
Contributor

@adrgs adrgs commented Jul 22, 2024

What is the problem that this fixes or functionality that this introduces? Does it fix any open issues?

This PR introduces the possibility of having a defined security analyzer watch over OpenDevin's actions and watch accordingly. It's integrated by default with confirmation mode, which analyzers can make use of by auto-approving actions deemed safe.

We have also implemented such an analyzer, using invariant


Give a summary of what the PR does, explaining any non-trivial design decisions

Backend

  • Important actions now have the security_risk field, which is Optional
  • The Security Analyzer lives in AgentSession, and can only be created at start-up if the proper flag is set
  • Added a /api/security/{path:path} API route, for the analyzers to be able to interact with the user without having to modify the core code for each
  • Also added /api/options/security-analyzers endpoint, which lists the agents
  • Added documentation in opendevin/security for creating new analyzers
  • The Invariant Analyzer runs in a separate Docker container, to not add any dependencies to OpenDevin

Frontend

  • Added new combobox / string setting for SECURITY_ANALYZER
  • Added a Lock icon in the bottom right that is visible when there's a security analyzer present
  • Added frontend/src/components/modals/security, a place where the modules can create their own modals

Other references
Video of how the changes look like:
https://www.youtube.com/watch?v=brH7CUGyzmI

mbalunovic and others added 30 commits June 28, 2024 18:20
Copy link
Copy Markdown
Collaborator

@xingyaoww xingyaoww left a comment

Choose a reason for hiding this comment

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

Backend LGTM! If frontend also look good to folks, i'd be have to approve it.

# Security

Given the impressive capabilities of OpenDevin and similar coding agents, ensuring robust security measures is essential to prevent unintended actions or security breaches. The SecurityAnalyzer framework provides a structured approach to monitor and analyze agent actions for potential security risks.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you add a section here as a quick start to show people how to enable this feature? e.g., on the UI and on the backend?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure, added it in the latest commit:

Suggested change
To enable this feature:
* From the web interface
* Open Configuration (by clicking the gear icon in the bottom right)
* Select a Security Analyzer from the dropdown
* Save settings
* (to disable) repeat the same steps, but click the X in the Security Analyzer dropdown
* From config.toml
```toml
[security]
# Enable confirmation mode
confirmation_mode = true
# The security analyzer to use
security_analyzer = "your-security-analyzer"
```
(to disable) remove the lines from config.toml

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Awesome!

@xingyaoww xingyaoww marked this pull request as ready for review August 8, 2024 17:20
@xingyaoww xingyaoww requested review from enyst and xingyaoww August 8, 2024 17:28
@xingyaoww
Copy link
Copy Markdown
Collaborator

Hey @adrgs, I'd be happy to merge it if you could resolve the conflict (i help did it in this PR invariantlabs-ai#1)

Merge main and resolve conflict
@adrgs
Copy link
Copy Markdown
Contributor Author

adrgs commented Aug 8, 2024

Hey @adrgs, I'd be happy to merge it if you could resolve the conflict (i help did it in this PR invariantlabs-ai#1)

thanks for the help @xingyaoww ! should be up to date with main now

@neubig
Copy link
Copy Markdown
Contributor

neubig commented Aug 12, 2024

Hey @xingyaoww , could you make the final call on this and we can get it merged in? This is exciting!

@xingyaoww xingyaoww enabled auto-merge (squash) August 13, 2024 07:09
Copy link
Copy Markdown
Collaborator

@xingyaoww xingyaoww left a comment

Choose a reason for hiding this comment

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

Let's get this in!

@xingyaoww xingyaoww disabled auto-merge August 13, 2024 07:11
@xingyaoww
Copy link
Copy Markdown
Collaborator

@adrgs sorry for the late response! Could you resolve the one last merge conflict: invariantlabs-ai#2

@adrgs
Copy link
Copy Markdown
Contributor Author

adrgs commented Aug 13, 2024

@xingyaoww added, thanks!

@neubig neubig enabled auto-merge (squash) August 13, 2024 10:09
@neubig neubig merged commit e0b67ad into OpenHands:main Aug 13, 2024
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.

5 participants