Skip to content

Conversation

@dorien-koelemeijer
Copy link
Collaborator

@dorien-koelemeijer dorien-koelemeijer commented Sep 16, 2025

PR Description

UI configuration for prompt injection detection

Allow users to enable prompt injection detection and the confidence threshold through the UI instead of having to manually update goose config file.

Screenshot 2025-09-16 at 11 53 38 am

Update security logging

  • Noticed that security findings didn't get assigned a unique id (😱), so fixed that (SEC-{uuid})
  • User decisions (allow/deny) are logged with this finding ID for easier log analysis

Some general cleanup

Remove some unnecessary/unused code

@dorien-koelemeijer dorien-koelemeijer marked this pull request as draft September 16, 2025 09:46
@dorien-koelemeijer dorien-koelemeijer force-pushed the feat/add-prompt-injection-detection-settings-ui branch 2 times, most recently from 6c7866a to 82a0c31 Compare September 16, 2025 10:35
@dorien-koelemeijer dorien-koelemeijer marked this pull request as ready for review September 16, 2025 10:37
@dorien-koelemeijer dorien-koelemeijer force-pushed the feat/add-prompt-injection-detection-settings-ui branch from 8cd5566 to 82a0c31 Compare September 16, 2025 13:58
@dorien-koelemeijer dorien-koelemeijer force-pushed the feat/add-prompt-injection-detection-settings-ui branch from 8393cf5 to 6562601 Compare September 16, 2025 18:07
@michaelneale michaelneale self-assigned this Sep 16, 2025
@dorien-koelemeijer dorien-koelemeijer changed the title Feat: Add prompt injection detection settings UI + update security logging Feat: Add prompt injection detection settings UI + update logging Sep 17, 2025
Copy link
Collaborator

@DOsinga DOsinga left a comment

Choose a reason for hiding this comment

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

I had a quick look, but I fear this feels rather vibe codey - there's some duplication, unrelated changes, I wonder if you could do a bit of a review more yourself?

@dorien-koelemeijer
Copy link
Collaborator Author

I had a quick look, but I fear this feels rather vibe codey - there's some duplication, unrelated changes, I wonder if you could do a bit of a review more yourself?

You're right, I wanted to get it done too quickly. Having another look now and will do some cleanup. Thanks for the comments

@dorien-koelemeijer dorien-koelemeijer force-pushed the feat/add-prompt-injection-detection-settings-ui branch 2 times, most recently from 4bd65ae to 1c89c0a Compare September 18, 2025 11:46
@dorien-koelemeijer dorien-koelemeijer force-pushed the feat/add-prompt-injection-detection-settings-ui branch from 1c89c0a to a79ec4e Compare September 18, 2025 11:47
@michaelneale michaelneale assigned DOsinga and unassigned michaelneale Sep 18, 2025
@dorien-koelemeijer dorien-koelemeijer force-pushed the feat/add-prompt-injection-detection-settings-ui branch from 56bad4b to f57511e Compare September 19, 2025 10:11
@dorien-koelemeijer dorien-koelemeijer force-pushed the feat/add-prompt-injection-detection-settings-ui branch from 9b5c660 to 7a9d9d4 Compare September 19, 2025 10:13
@dorien-koelemeijer dorien-koelemeijer force-pushed the feat/add-prompt-injection-detection-settings-ui branch from bd7293a to 0ae76d5 Compare September 19, 2025 11:03
@michaelneale
Copy link
Collaborator

looks like linting issues now

@michaelneale
Copy link
Collaborator

so the config is now saved as:

security_threshold: 0.2
security_enabled: true

not in a security: section - is that expected? (also I noted it didnt' save the threshold until I changed it, which I assume means default?)

@dorien-koelemeijer there are a few other changes not GUI related, what is best way to check this is still healthy?

looks ok otherwise, @DOsinga happier with shape of code?

@michaelneale
Copy link
Collaborator

thread safety/oncelock stuff looks good too

@dorien-koelemeijer
Copy link
Collaborator Author

so the config is now saved as:

security_threshold: 0.2
security_enabled: true

not in a security: section - is that expected? (also I noted it didnt' save the threshold until I changed it, which I assume means default?)

@dorien-koelemeijer there are a few other changes not GUI related, what is best way to check this is still healthy?

looks ok otherwise, @DOsinga happier with shape of code?

It'll be security.threshold: xx and security.enabled: true/false. But I think most people will just go through the UI settings now, since it's a lot easier.

Re: non-GUI related changes, you can simply test the changes in goose CLI/desktop version locally. I've tested until the last lint commit and everything was still fine - will do some final testing in the next couple of hours, but it should all be good.

@michaelneale
Copy link
Collaborator

@dorien-koelemeijer but when I saw it in the config.yaml - it wasn't grouped under security (was an underscore) - just wanted to know if that was expected?

@dorien-koelemeijer
Copy link
Collaborator Author

@dorien-koelemeijer but when I saw it in the config.yaml - it wasn't grouped under security (was an underscore) - just wanted to know if that was expected?

Really? Let me have a look as well 👀 In any case, I'll have to update instructions once this is merged. Would be great to get both this PR and the other one together in one release to prevent confusion

@dorien-koelemeijer
Copy link
Collaborator Author

dorien-koelemeijer commented Sep 24, 2025

@dorien-koelemeijer but when I saw it in the config.yaml - it wasn't grouped under security (was an underscore) - just wanted to know if that was expected?

Really? Let me have a look as well 👀 In any case, I'll have to update instructions once this is merged. Would be great to get both this PR and the other one together in one release to prevent confusion

I think I tried to keep things backwards compatible first and still had that config saved - you're right, it's definitely with underscores

Screenshot 2025-09-24 at 11 42 44 am

Seems like it would make sense to keep underscores, since all config is saved like that? I guess it depends on whether this PR gets released at the same time as the other one, then we don't have to have things be backwards compatible. Honestly, probably easiest to keep it in line with all other config though and use underscores?

@dorien-koelemeijer
Copy link
Collaborator Author

so the config is now saved as:

security_threshold: 0.2
security_enabled: true

not in a security: section - is that expected? (also I noted it didnt' save the threshold until I changed it, which I assume means default?)
@dorien-koelemeijer there are a few other changes not GUI related, what is best way to check this is still healthy?
looks ok otherwise, @DOsinga happier with shape of code?

It'll be security.threshold: xx and security.enabled: true/false. But I think most people will just go through the UI settings now, since it's a lot easier.

Re: non-GUI related changes, you can simply test the changes in goose CLI/desktop version locally. I've tested until the last lint commit and everything was still fine - will do some final testing in the next couple of hours, but it should all be good.

Have tested both desktop and CLI version - all seems fine still. Testing will be the same as for the other PR if you wanted to give it a go as well to be safe

Copy link
Collaborator

@DOsinga DOsinga left a comment

Choose a reason for hiding this comment

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

Looks much better, yeah! I think you can delete even more LLM comments - I always do

@michaelneale michaelneale self-assigned this Sep 30, 2025
@DOsinga DOsinga merged commit e6a5692 into block:main Oct 3, 2025
10 checks passed
wpfleger96 added a commit to wpfleger96/goose that referenced this pull request Oct 3, 2025
* main:
  docs: Change community page sections (block#4984)
  docs: remove temporary Hacktoberfest issue templates (block#4982)
  Create multi-channel researcher prompt (block#4947)
  docs: Add Community Content section to Community Page (block#4964)
  Allow empty API Key when registering custom provider (block#4977)
  Feat: Add prompt injection detection settings UI + update logging (block#4651)
  Make create_session work concurrently (block#4954)
  Lifei/create save recipe to file (block#4895)
Itz-Agasta pushed a commit to Itz-Agasta/goose that referenced this pull request Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants