feat: implement a tool permission store #1516
Conversation
wendytang
changed the title
|
|
||
| let file = File::open(file_path)?; | ||
| let mut permissions: ToolPermissionStore = serde_json::from_reader(file)?; | ||
| permissions.permissions_dir = store.permissions_dir; |
There was a problem hiding this comment.
do we need to clean up the expired entries?
| if let Ok(tool_call) = request.tool_call.clone() { | ||
| if let Some(allowed) = store.check_permission(request) { | ||
| if allowed { | ||
| let output = capabilities.dispatch_tool_call(tool_call).await; |
There was a problem hiding this comment.
do we need to output something to let users know the permission is skipped due to cache?
There was a problem hiding this comment.
possibly, it could be a bit noisy imo
| // This helps identify when the same tool is being used in a different context | ||
| let mut hasher = Hasher::new(); | ||
| hasher.update( | ||
| serde_json::to_string(&tool_request.tool_call.as_ref().unwrap().arguments) |
There was a problem hiding this comment.
do we want to hash on some argument params? I am wondering whether we will have low hit rate if we hash all, including argument param and value, like write file, the name can be different, but they are super similar
There was a problem hiding this comment.
Similar thought - I was thinking to hash by tool name only at first, but this would lump all bash commands in one hash.
Any ideas about how to do the hash at the right level of granularity?
There was a problem hiding this comment.
yeah, kind of tricky, introducing some cosine similarity check may be overkilled. I am wondering whether we can have a set of normalized keys? for example in bash case, we have "command":write, "file_path": xxx, and we maintain a set containing "file_path", if it is in the normalized key set, like "file_path", we replace the value with "normalized_value" before hashing. Even with such case, we cannot handle all argument keys, we can just start with bash? how do you think?
|
Overall, lgtm, thanks for the improvment |
~/.config/goose/tool_permissions.json