-
Notifications
You must be signed in to change notification settings - Fork 664
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
DuckDuckGo + Filesystem Management #9
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some concerns about search (see Slack) and filesystem servers generally, although I like the idea of both. Let's chat about it.
const url = `https://html.duckduckgo.com/html/?q=${encodeURIComponent(query)}`; | ||
const headers = { | ||
"User-Agent": | ||
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.124 Safari/537.36", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use a User-Agent that identifies this code specifically, so DDG could track where it's coming from if needed?
@@ -0,0 +1,76 @@ | |||
# Filesystem MCP Server | |||
|
|||
Node.js server implementing Model Context Protocol (MCP) for filesystem operations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of this server, but it's a bit of a security nightmare right now. I think we should lock it down so that it only makes available specific directories on disk (e.g., passed as CLI arguments), and then we have to be really careful with all of our fs operations checks to ensure that they're happening entirely within that directory (accounting for symlinks and other weirdness too).
Does that sound reasonable? I can help out here if it'd be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think that makes sense - Pietro & I chatted a bit about this last night as well.
@Skirano do you have thoughts on this? If you agree, would you mind updating this (or working with Justin) such that it only works for specific directories that get passed in? Sorry for all the back and forth here!
Re: duckduckgo - understand your concern. I'll defer to the slack thread's outcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I can add that!
No description provided.