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

feat(agent): Add URL whitelisting/blacklisting to config and URL validation #6848

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Satyam97
Copy link

Background

This PR refers to the solution for enhancement suggested here: #5289

This basically takes account white/black listing of URLs during context setting in agents.

Changes πŸ—οΈ

I have added 2 env variables for web policy and URL list. With a combination of both, a URL is white or blacklisted by the read_webpage function.

PR Quality Scorecard ✨

  • Have you used the PR description template?   +2 pts
  • Is your pull request atomic, focusing on a single change?   +5 pts
  • Have you linked the GitHub issue(s) that this PR addresses?   +5 pts
  • Have you documented your changes clearly and comprehensively?   +5 pts
  • Have you changed or added a feature?   -4 pts
    • [x ] Have you added/updated corresponding documentation?   +4 pts
    • Have you added/updated corresponding integration tests?   +5 pts
  • Have you changed the behavior of AutoGPT?   -5 pts
    • Have you also run agbenchmark to verify that these changes do not regress performance?   +10 pts

Copy link

netlify bot commented Feb 14, 2024

βœ… Deploy Preview for auto-gpt-docs ready!

Name Link
πŸ”¨ Latest commit 27b1b4e
πŸ” Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/65cd180460115f0008405bfe
😎 Deploy Preview https://deploy-preview-6848--auto-gpt-docs.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ntindle
Copy link
Member

ntindle commented Feb 15, 2024

remove falcon from the PR

### White/Black Listing URLS
################################################################################
##WEB_POLICY - Specifies whether the url list is black list(0) or a whilte list(1)
#WEB_POLICY=0

Choose a reason for hiding this comment

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

I think we need a policy for users who want to continue using AutoGPT as normal. This looks like it forces them to choose between whitelisting and blacklisting, which some users might not want either. There needs to be a default option that simply disables this feature.

##WEB_POLICY - Specifies whether the url list is black list(0) or a whilte list(1)
#WEB_POLICY=0
##URL_LIST - Comma separated URLs to be black or whitelisted, sets according to WEB_POLICY value
#URL_LIST=https://www.google.com,http://www.google.com

Choose a reason for hiding this comment

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

The only thing I don't like about putting it here, is that these lists might get pretty long. That's why in my version I was attempting to parse files.

Copy link
Member

@Pwuts Pwuts left a comment

Choose a reason for hiding this comment

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

This still needs some work and testing before we can consider merging it

arena/falcon.json Show resolved Hide resolved
Comment on lines +243 to +244
##WEB_POLICY - Specifies whether the url list is black list(0) or a whilte list(1)
#WEB_POLICY=0
Copy link
Member

Choose a reason for hiding this comment

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

  • Formatting diverges from the rest of the file
  • Rather make this an enum with values blacklist and whitelist

Comment on lines +244 to +246
# White/Black Listing URLs
web_policy: Optional[str] = os.getenv("WEB_POLICY", 0)
url_list: Optional[list] = os.getenv("URL_LIST",[]).split(",")
Copy link
Member

Choose a reason for hiding this comment

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

This is not how the config of the application works, as you can see from how all the other config attributes are defined.


P = ParamSpec("P")
T = TypeVar("T")

config = Config
Copy link
Member

Choose a reason for hiding this comment

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

this won't work

Copy link
Author

Choose a reason for hiding this comment

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

@Pwuts can you please help with why this wont work?

Copy link
Author

Choose a reason for hiding this comment

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

from autogpt.config import ConfigBuilder
config = ConfigBuilder.build_config_from_env()

web_policy = config.web_policy
url_list = config.url_list

will this work? @Pwuts

@@ -2,10 +2,11 @@
import re
from typing import Any, Callable, ParamSpec, TypeVar
from urllib.parse import urljoin, urlparse
from autogpt.config import Config
Copy link
Member

Choose a reason for hiding this comment

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

this is gonna throw a linting error

Copy link
Author

Choose a reason for hiding this comment

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

Added flake8 to VS code, sorry there was some issue with flake earlier.

Comment on lines +46 to +48
raise ValueError("URL Not Whitelisted")
elif url in url_list:
raise ValueError("URL Blacklisted.")
Copy link
Member

Choose a reason for hiding this comment

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

inconsistent formatting

Copy link
Author

@Satyam97 Satyam97 Feb 16, 2024

Choose a reason for hiding this comment

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

I have fixed the linting this update the PR with the above config changes @Pwuts

@Pwuts Pwuts marked this pull request as draft February 16, 2024 15:20
@Pwuts Pwuts changed the title Validation feat(agent): Add URL whitelisting/blacklisting to config and URL validation Feb 16, 2024
@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Feb 28, 2024
Copy link

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoGPT Agent conflicts Automatically applied to PRs with merge conflicts function: browse size/m
Projects
Status: 🚧 Needs work
Development

Successfully merging this pull request may close these issues.

None yet

5 participants