Feat: Agent API Origin Whitelisting #2062
Conversation
|
@ardafincan is attempting to deploy a commit to the Arc53 Team on Vercel. A member of the Team first needs to authorize it. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2062 +/- ##
==========================================
+ Coverage 42.28% 42.64% +0.35%
==========================================
Files 136 137 +1
Lines 9315 9478 +163
==========================================
+ Hits 3939 4042 +103
- Misses 5376 5436 +60 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ardafincan
changed the title
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Hey @dartpain, you have any time to review this one? |
| agent = agents_collection.find_one({"key": api_key}) | ||
|
|
||
| request_origin = request.headers.get("Origin") | ||
| granted_origins = json.loads(agent.get("granted_origins", "[]")) |
There was a problem hiding this comment.
This would crash with AttributeError if agent is not found, could add a null check like in check_usage()
|
|
||
| if granted_origins == []: | ||
| return None | ||
| elif request_origin in granted_origins or (str(request_origin)+"/") in granted_origins: |
There was a problem hiding this comment.
Trailing slashes and case differences might cause valid requests to fail. Might want to normalize the URLs before comparing them.
| return None | ||
|
|
||
| return make_response( | ||
| jsonify({"success": "True", "message": "You are not authorized for this action."}), |
There was a problem hiding this comment.
"True"
Should make this boolean for consistency.
| value={originInput} | ||
| onChange={(e) => setOriginInput(e.target.value)} | ||
| placeholder="Enter origin URL (https://example.com)" | ||
| pattern="^https|http://.+" |
There was a problem hiding this comment.
Wouldn't ^https?://.+ be a better regex to use?
| /> | ||
| <button | ||
| type="button" | ||
| onClick={() => { |
There was a problem hiding this comment.
Consider refactoring this into a shared handleAddOrigin() as there is duplicated code just before.
| data["tools"] = [] | ||
| if "sources" in data: | ||
| try: | ||
| data["sources"] = json.loads(data["sources"]) |
There was a problem hiding this comment.
Add granted_origins to early JSON parsing (like sources/tools):
if "granted_origins" in data:
try:
data["granted_origins"] = json.loads(data["granted_origins"])
except json.JSONDecodeError:
data["granted_origins"] = []| data = request.get_json() | ||
| else: | ||
| data = request.form.to_dict() | ||
| json_fields = ["tools", "sources", "json_schema"] |
There was a problem hiding this comment.
Add "granted_origins" to json_fields list for early parsing:
json_fields = ["tools", "sources", "json_schema", "granted_origins"]
This ensures consistency with how other JSON fields are handled.
| <div className="space-y-2"> | ||
| {agent.granted_origins.map((origin, index) => ( | ||
| <div | ||
| key={index} |
There was a problem hiding this comment.
Could improve it by using a unique key instead of index keys, maybe key={origin}?
|
Hi @ardafincan thanks for the PR! |
|
Hey @siiddhantt, thanks for your detailed review. I am a bit busy this week, but I will handle the changes and test additions you wanted as soon as possible. |
|
No worries @ardafincan thanks for contributing! |
2 participants
This PR introduces agent API Origin Whitelisting feature for security purposes. User can define allowed origins for accessing agents via API calls.
Issue 🚀 Feature: Agent API security improvements #1984 explains the need of this feature, it can be summarized as security improvement.