Skip to content

refactor: Simplify agent request store #15743

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

schrothbn
Copy link
Contributor

Summary

This PR simplifies the agent request store and removes unneeded code. The store is now more focused on its intend and doesn't need complex logic to build the object that's being sent to the backend. It is making now use of useLocalStorage instead of handling that inside the store and naming of functions have been adjusted. This should also address a potential security vulnerability caused by the old handling.

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/AI-950/prototype-pollution-vulnerability-detected

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

@schrothbn schrothbn requested a review from a team May 27, 2025 11:20
@schrothbn schrothbn self-assigned this May 27, 2025
@schrothbn schrothbn requested review from burivuhster and removed request for a team May 27, 2025 11:20
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

cubic found 2 issues across 7 files. Review them in cubic.dev

React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.

@n8n-assistant n8n-assistant bot added the n8n team Authored by the n8n team label May 27, 2025
@schrothbn schrothbn force-pushed the ai-950-prototype-pollution-vulnerability-detected branch from d577967 to c4510f2 Compare May 27, 2025 12:06
Copy link

codecov bot commented May 27, 2025

Codecov Report

Attention: Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...editor-ui/src/components/FromAiParametersModal.vue 82.35% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Contributor

@burivuhster burivuhster left a comment

Choose a reason for hiding this comment

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

Nice work! Code looks much more clear now. I had only one small question/comment, feel free to merge as is if it's not relevant.

};
};

const clearAgentRequests = (workflowId: string, nodeId: string): void => {
if (agentRequests.value[workflowId]) {
agentRequests.value[workflowId][nodeId] = {};
agentRequests.value[workflowId][nodeId] = { query: {} };
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just delete it instead of setting to empty content here? And maybe even delete agentRequests.value[workflowId] as well if it was the last key?

@shortstacked
Copy link
Contributor

Workflow Test Results 📊 ⚠️ 4 Warnings (0 Failed), 👍 79 Successful out of 83 total workflows.

View full workflow run

Tested Ref: c4510f202024bb7da60be06168877b169dd1f835 by @burivuhster

⚠️ Warnings (4)

Workflow ID Workflow Name Reason
237 BasicLLMChain:AzureChat Workflow contains new data that previously did not exist.
35 Slack:User:getPresence info:UserProfile:get update... Workflow contains new data that previously did not exist.
257 Agent:auto-fix:anthropic Workflow contains new data that previously did not exist.
53 ConvertKit:CustomField:create getAll update delete... Workflow contains new data that previously did not exist.

Copy link
Contributor

⚠️ Some Cypress E2E specs are failing, please fix them before merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
n8n team Authored by the n8n team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants