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

APIChain add restrictions to domains (CVE-2023-32786) #12747

Merged
merged 5 commits into from
Nov 1, 2023

Conversation

eyurtsev
Copy link
Collaborator

@eyurtsev eyurtsev commented Nov 1, 2023

  • Restrict the chain to specific domains by default
  • This is a breaking change, but it will fail loudly upon object instantiation -- so there should be no silent errors for users
  • Resolves CVE-2023-32786

Copy link

vercel bot commented Nov 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Nov 1, 2023 9:49pm

@eyurtsev eyurtsev requested a review from baskaryan November 1, 2023 21:37
@dosubot dosubot bot added the 🤖:improvement Medium size change to existing code to handle new use-cases label Nov 1, 2023
@eyurtsev eyurtsev changed the title APIChain add restrictions to domains APIChain add restrictions to domains (CVE-2023-32786) Nov 1, 2023
Copy link
Collaborator

@baskaryan baskaryan left a comment

Choose a reason for hiding this comment

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

one nit

@@ -40,6 +73,19 @@ class APIChain(Chain):
api_docs: str
question_key: str = "question" #: :meta private:
output_key: str = "output" #: :meta private:
limit_to_domains: Optional[Sequence[str]] = ()
Copy link
Collaborator

Choose a reason for hiding this comment

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

doe we need a default value? we raise a value if one's not explicitly specified anyways

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated -- but no functional change either way

@eyurtsev eyurtsev requested a review from baskaryan November 1, 2023 21:51
@eyurtsev eyurtsev merged commit b1caae6 into master Nov 1, 2023
23 checks passed
@eyurtsev eyurtsev deleted the eugene/api_chain branch November 1, 2023 22:50
xieqihui pushed a commit to xieqihui/langchain that referenced this pull request Nov 21, 2023
…2747)

* Restrict the chain to specific domains by default
* This is a breaking change, but it will fail loudly upon object
instantiation -- so there should be no silent errors for users
* Resolves CVE-2023-32786
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:improvement Medium size change to existing code to handle new use-cases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants