Desktop alerts when suspicious unicode characters found in Recipe #4080
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
amed-xyz
force-pushed
the
amed/recipe-security-alert
branch
5 times, most recently
from
89a1fc2 to
9c06bca
Compare
DOsinga
approved these changes
Aug 15, 2025
Collaborator
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
the screenshot looks great. there's a bunch of oddities in the code though we could address...
| assert_eq!(extensions.len(), 0); | ||
| } | ||
|
|
||
| #[test] |
Collaborator
There was a problem hiding this comment.
this test strikes me as too trivial to do any good
Collaborator
Author
There was a problem hiding this comment.
just making sure we're not bypassing any of the places we want to look into
| onConfirm, | ||
| onCancel, | ||
| recipeDetails, | ||
| hasSecurityWarnings = false, |
Collaborator
Author
There was a problem hiding this comment.
assume there's no security warnings until we run the check and find any
| console.error('Failed to scan recipe:', error); | ||
| throw error; | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
why do we need this wrapper? we call this once and we have the generated code
Collaborator
Author
There was a problem hiding this comment.
no strong reason, was just following same pattern as the other endpoints
| } catch (error) { | ||
| console.error('Error checking recipe acceptance:', error); | ||
| // If there's an error, assume the recipe hasn't been accepted | ||
| setHasSecurityWarnings(false); |
Collaborator
There was a problem hiding this comment.
if there is an error here, we show the dialog but never the secuirty warning, even if the scan failed?
Collaborator
Author
There was a problem hiding this comment.
yes, basically we default to a generic warning (current behaviour) instead of assuming there's an actual security warning and saying so
amed-xyz
force-pushed
the
amed/recipe-security-alert
branch
2 times, most recently
from
b294618 to
0529a4f
Compare
amed-xyz
force-pushed
the
amed/recipe-security-alert
branch
from
0529a4f to
4e0720e
Compare
zanesq
added a commit
that referenced
this pull request
Aug 19, 2025
…-visual-improvements * 'main' of github.com:block/goose: (21 commits) Custom providers update (#4099) docs: goose_terminal env var (#4205) Desktop alerts when suspicious unicode characters found in Recipe (#4080) chore: remove the google drive built-in extension (#4187) Move out app init (#4185) Remove unused extension stuff (#4166) Added tests for extensions functionality (#3794) chore(release): release version 1.5.0 (#4169) Fix tests from upstream changes and add testing to lint staged and ci (#4127) Unlist figma tutorial (#4186) feat(ui): Implement in-place message editing with re-response (#3798) Retry all 500 codes (#4160) blog: Transforming AI Assistance with Goose Mentor Mode (#4151) upgraded all npm packages and fixed related issues (#4072) Docs: @-mentions in goosehints (#4171) fix: consistent font sizing in ToolCallWithResponse (#4167) Temporarily disable TODO Tool (#4158) docs: add integrated MCP server config to jetbrains tutorial (#4120) docs: remove figma MCP from suggested servers (#4123) Blog: The AI Skeptic’s Guide to Context Windows (#4152) ...
ayax79
pushed a commit
to ayax79/goose
that referenced
this pull request
Aug 21, 2025
…ock#4080) Signed-off-by: Jack Wright <[email protected]>
michaelneale
added a commit
that referenced
this pull request
Aug 21, 2025
* main: docs: add figma tutorial (#4231) Add Nix flake for reproducible builds (#4213) Enhanced onboarding page visual design (#4156) feat: adds mtls to all providers (#2794) (#2799) Don't show a confirm dialog for quitting (#4225) Fix: Missing smart_approve in CLI /mode help text and error message (#4132) Clean up langfuse docs and scripts (#4220) feat: add remark-breaks plugin to preserve single newlines in markdown (#4217) feat(mcp/developer): accept -1 for insert_line number (#4112) Remove dead code and old settings migration (#4180) removed tests from lint-staged (#4203) docs: openrouter and ollama easy desktop setup (#4195) Custom providers update (#4099) docs: goose_terminal env var (#4205) Desktop alerts when suspicious unicode characters found in Recipe (#4080) chore: remove the google drive built-in extension (#4187) Move out app init (#4185)
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up from
Detect potentially harmful content in recipes upon loading and alert users through the existing warning modal system.
When Recipe with potentially harmful content is loaded
Triggered on new Recipes only.
When Recipe without potentially harmful content is loaded
Existing behaviour preserved: only triggered on new Recipes.