🛡️ refactor: Scope Action Mutations by Parent Resource Ownership#12237
Merged
Conversation
Prevent cross-tenant action overwrites by validating that an existing action's agent_id/assistant_id matches the URL parameter before allowing updates or deletes. Without this, a user with EDIT access on their own agent could reference a foreign action_id to hijack another agent's action record.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the Agent/Assistant action mutation routes to prevent cross-tenant overwrites by ensuring an existing action_id is scoped to the parent resource (agent_id / assistant_id) before allowing updates or deletes.
Changes:
- Add parent-ownership checks when updating an existing action via
POST /actions/:{agent|assistant}_id. - Scope action deletions by passing
{ action_id, agent_id }/{ action_id, assistant_id }intodeleteAction. - Return a 403 when an action is detected as belonging to a different parent resource.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| api/server/routes/assistants/actions.js | Adds assistant ownership validation for action updates; scopes action deletion by assistant_id. |
| api/server/routes/agents/actions.js | Adds agent ownership validation for action updates; scopes action deletion by agent_id. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
|
||
| if (actions_result && actions_result.length) { | ||
| const action = actions_result[0]; | ||
| if (action.assistant_id && action.assistant_id !== assistant_id) { |
Comment on lines
201
to
203
| promises.push(updateAssistantDoc({ assistant_id }, assistantUpdateData)); | ||
| promises.push(deleteAction({ action_id })); | ||
| promises.push(deleteAction({ action_id, assistant_id })); | ||
|
|
|
|
||
| if (actions_result && actions_result.length) { | ||
| const action = actions_result[0]; | ||
| if (action.agent_id && action.agent_id !== agent_id) { |
Comment on lines
256
to
258
| ); | ||
| await deleteAction({ action_id }); | ||
| await deleteAction({ action_id, agent_id }); | ||
| res.status(200).json({ message: 'Action deleted successfully' }); |
- Remove && short-circuit that bypassed the guard when agent_id or assistant_id was falsy (e.g. assistant-owned actions have no agent_id, so the check was skipped entirely on the agents route). - Include agent_id / assistant_id in the updateAction and deleteAction query filters so the DB write itself enforces ownership atomically. - Log a warning when deleteAction returns null (silent no-op from data-integrity mismatch).
Cover update, delete, and cross-type protection scenarios using MongoMemoryServer to verify that scoped query filters (agent_id, assistant_id) prevent cross-tenant overwrites and deletions at the database level.
danny-avila
changed the title
…ng agent actions The duplicate handler was splitting `action.action_id` by `actionDelimiter` to extract the domain, but `action_id` is a bare nanoid that doesn't contain the delimiter. This produced malformed entries in the duplicated agent's actions array (nanoid_action_newNanoid instead of domain_action_newNanoid). The domain is available on `action.metadata.domain`.
Uses MongoMemoryServer with real Agent and Action models to verify: - Duplicated actions use metadata.domain (not action_id) for the agent actions array entries - Sensitive metadata fields are stripped from duplicated actions - Original action documents are not modified
danny-avila
changed the title
jcbartle
pushed a commit
to jcbartle/LibreChat
that referenced
this pull request
May 11, 2026
…ny-avila#12237) * 🛡️ fix: Scope action mutations by parent resource ownership Prevent cross-tenant action overwrites by validating that an existing action's agent_id/assistant_id matches the URL parameter before allowing updates or deletes. Without this, a user with EDIT access on their own agent could reference a foreign action_id to hijack another agent's action record. * 🛡️ fix: Harden action ownership checks and scope write filters - Remove && short-circuit that bypassed the guard when agent_id or assistant_id was falsy (e.g. assistant-owned actions have no agent_id, so the check was skipped entirely on the agents route). - Include agent_id / assistant_id in the updateAction and deleteAction query filters so the DB write itself enforces ownership atomically. - Log a warning when deleteAction returns null (silent no-op from data-integrity mismatch). * 📝 docs: Update Action model JSDoc to reflect scoped query params * ✅ test: Add Action ownership scoping tests Cover update, delete, and cross-type protection scenarios using MongoMemoryServer to verify that scoped query filters (agent_id, assistant_id) prevent cross-tenant overwrites and deletions at the database level. * 🛡️ fix: Scope updateAction filter in agent duplication handler * 🐛 fix: Use action metadata domain instead of action_id when duplicating agent actions The duplicate handler was splitting `action.action_id` by `actionDelimiter` to extract the domain, but `action_id` is a bare nanoid that doesn't contain the delimiter. This produced malformed entries in the duplicated agent's actions array (nanoid_action_newNanoid instead of domain_action_newNanoid). The domain is available on `action.metadata.domain`. * ✅ test: Add integration tests for agent duplication action handling Uses MongoMemoryServer with real Agent and Action models to verify: - Duplicated actions use metadata.domain (not action_id) for the agent actions array entries - Sensitive metadata fields are stripped from duplicated actions - Original action documents are not modified
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
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.
Summary
Fixed a cross-tenant authorization vulnerability where a user with EDIT access on their own agent or assistant could reference a foreign
action_idto overwrite or hijack another resource's action record.&&short-circuit from the ownership guard on the POST (update) path in both the agents and assistants action routes, which silently bypassed the check for any action whoseagent_idorassistant_idwas falsy — including all cross-type records (e.g., an assistant-owned action has noagent_id, so the guard on the agents route never fired).agent_idandassistant_idinto theupdateActionanddeleteActionquery filters so DB-level ownership is enforced atomically with the write, eliminating the check-then-act race between the pre-check fetch and the write operation.logger.warnwhendeleteActionreturnsnullin both DELETE routes, surfacing silent partial failures caused by data-integrity skew between the agent/assistant document's actions array and the Action collection.updateActionanddeleteActionJSDoc inapi/models/Action.jsto accurately reflect the scoped query parameter shapes (agent_id?,assistant_id?) instead of the now-staleuserfield.api/models/Action.spec.jsusingMongoMemoryServerand real Mongoose models, covering matching and mismatchedagent_id/assistant_idfor both update and delete operations,getActionsscoping, and cross-type protection (agent filter cannot overwrite or delete an assistant-owned action and vice versa).Change Type
Testing
Tests are written as model-layer integration tests in
api/models/Action.spec.jsusingmongodb-memory-serverwith real Mongoose models and no mocking of DB logic, per project testing philosophy.Test Configuration:
12 tests across 4
describeblocks:updateAction— matching and mismatchedagent_id/assistant_iddeleteAction— matching and mismatchedagent_id/assistant_idgetActions— unscoped and scoped baselinesChecklist