[Security] Implement Security Option for Handling Tool-Call Chains #4691

shellz-n-stuff · 2025-09-22T01:09:14Z

Pull Request Description

One of the key attacks against Goose is MCP poisoning. Using the response data from an MCP to trigger a non-expected tool-call that performs an unintended action.

Whilst we absolutely don't want to remove the utility of the agent autonomously deciding to call a chain of tools, users should be able to configure security profiles in their config to prevent certain tools being called as a "secondary tool" without user input.

This PR introduces such functionality

Implementation Detail

The general approach is to intercept tool-calls and perform a message look back to see what tools have been called since prior invocation. Specifically, the method is as follows:

Check for config of tools to protect (if null skip the check)
Check if the current tool-call exists in the list of protected tools (if null skip)
Collect a list of tool calls between the tool we're about to invoke and the last user message (if 0 skip)
If the last called tool is this tool skip
If the last called tool is another tool raise a security alert.

Testing

Config File Setup

Base Case Running a Normal MCP + The developer__shell mcp - Part 1

Base Case Running a Normal MCP + The developer__shell mcp - Part 2

Secondary Case running two instances of developer shell

…tool-calls

DOsinga

overall looks great, there's a lot of debugging logs in there that presumably we don't need anymore, can you clear those up? same for the LLM style comments

DOsinga · 2025-09-23T13:38:32Z

crates/goose/src/security/scanner.rs

 use serde_json::Value;

+/// Result of a security scan.
 #[derive(Debug, Clone)]


DOsinga · 2025-09-23T13:39:37Z

crates/goose/src/security/scanner.rs

    }

-    /// Get threshold from config
+    /// Get the maliciousness threshold from config, or use default.


if you're not happy with the current comment, rename the function (drop from_config, I don't think the caller cares where we store this)

DOsinga · 2025-09-23T13:48:27Z

crates/goose/src/security/scanner.rs

    }

-    /// Scan with prompt injection model (legacy method name for compatibility)
+    /// Scan text for prompt injection (legacy compatibility).


ugh, we should have caught this the first place. remove this comment - legacy

DOsinga · 2025-09-23T14:32:54Z

crates/goose/src/security/scanner.rs

+        disabled_secondary_tool_list: &[String],
+    ) -> bool {
+        let tool_name = tool_call.name.as_str();
+        if !disabled_secondary_tool_list.iter().any(|t| t == tool_name) {


I'd consider having the caller check this; that would leave this function with just one job, makes naming easier too

I was thinking of just making it fully self-contained (IE it "just works").

I may split this into into a function though to make it cleaner/improve readability

Actually looking at it, we re-use the tool_name var quite a lot. So I think it's really a call on if the caller should do it vs the function.

I personally like the function having the check but happy to change if you feel strongly

you should keep the toolname of course. I can go either way on it, but the function is called is_secondary_tool_violation_single, not is_secondary_tool_violation_single_if_enabled

DOsinga · 2025-09-23T14:36:11Z

crates/goose/src/security/scanner.rs

+                    }
+                }
+            }
+        }


you're going through the message twice here, I think you can do it in one go like:

for msg in messages.iter().rev():
if isUsere(msg): break
if toolCallName(msg) != toolCallName: return true
return true

also, we have effective_role which will return tool for tool messages

michaelneale · 2025-09-23T22:51:50Z

config.yaml makes sense as we already have it for rules for commands, but in future I wonder if people will want to build a distro with these more role-based (ie protect people from themselves) but I digress... looking good so far!

michaelneale · 2025-09-24T04:47:55Z

LGTM - but will want to check if this will be a problem with config.yaml:

change here #4651 - does this imply the config section in config.yaml is changing cc @dorien-koelemeijer ? (ie no security: section?)

dorien-koelemeijer · 2025-09-24T10:42:08Z

LGTM - but will want to check if this will be a problem with config.yaml:

change here #4651 - does this imply the config section in config.yaml is changing cc @dorien-koelemeijer ? (ie no security: section?)

Yeah, perhaps a comment on this whether we could also configure all of this through the UI settings and keep the config as all other config (with underscores)? See #4651 @shellz-n-stuff

shellz-n-stuff · 2025-09-24T18:19:07Z

#4651

@dorien-koelemeijer, I can make the updates on this branch after your PR gets merged? Not in a mega rush here!

dorien-koelemeijer · 2025-09-25T06:18:46Z

#4651

@dorien-koelemeijer, I can make the updates on this branch after your PR gets merged? Not in a mega rush here!

Just wondering whether you wanted me to update/rename any config items or not?

DOsinga · 2025-09-29T13:47:21Z

crates/goose/src/security/scanner.rs

-    /// Get threshold from config
-    pub fn get_threshold_from_config(&self) -> f32 {
+    /// Get the confidence threshold from config, or use default.
+    pub fn get_confidence_threshold_from_config(&self) -> f32 {


I thought we'd just call this get_confidence_threshold and drop the comment?

dorien-koelemeijer · 2025-10-03T14:04:44Z

Just adding a comment for visibility instead of chatting over Slack - I've now updated the config for prompt injection to be as follows: security_prompt_enabled and security_prompt_threshold, so perhaps you could update your config items to follow the same convention? Decided for the non-nested, underscore approach since that seems to be the more conventional thing to do looking at the other config. Happy to discuss more ofc if you wanted me to update something!

DOsinga · 2025-10-06T16:12:43Z

do we still want this to land?

DOsinga · 2025-10-18T21:27:58Z

I'm going to close this for now due to staleness. can always reopen when relevant again

shellz-n-stuff added 3 commits September 21, 2025 18:03

fix: Implement rules to better secure against malicious chaining for …

2ddb7f3

…tool-calls

fix: remove unused code and cleanup logging

f3903ec

fix: test coverage

c65ceee

shellz-n-stuff changed the title ~~[WIP][Security] Implement Security Option for Handling Tool-Call Chains~~ [Security] Implement Security Option for Handling Tool-Call Chains Sep 23, 2025

fix: linter

58ec9fd

shellz-n-stuff marked this pull request as ready for review September 23, 2025 00:29

michaelneale self-assigned this Sep 23, 2025

DOsinga approved these changes Sep 23, 2025

View reviewed changes

michaelneale assigned DOsinga Sep 23, 2025

fix: address review feedback

63d2c9c

michaelneale approved these changes Sep 24, 2025

View reviewed changes

michaelneale removed their assignment Sep 24, 2025

DOsinga reviewed Oct 2, 2025

View reviewed changes

DOsinga added the waiting label Oct 6, 2025

DOsinga closed this Oct 18, 2025

[Security] Implement Security Option for Handling Tool-Call Chains #4691

[Security] Implement Security Option for Handling Tool-Call Chains #4691

Uh oh!

Conversation

shellz-n-stuff commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Description

Implementation Detail

Testing

Uh oh!

DOsinga left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelneale commented Sep 23, 2025

Uh oh!

michaelneale commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dorien-koelemeijer commented Sep 24, 2025

Uh oh!

shellz-n-stuff commented Sep 24, 2025

Uh oh!

dorien-koelemeijer commented Sep 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dorien-koelemeijer commented Oct 3, 2025

Uh oh!

DOsinga commented Oct 6, 2025

Uh oh!

DOsinga commented Oct 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shellz-n-stuff commented Sep 22, 2025 •

edited

Loading

michaelneale commented Sep 24, 2025 •

edited

Loading