Enforce resource access during workflow execution#152
Conversation
|
Thanks for putting this together 🙏 This looks like a real and worthwhile access-control fix, and I agree with the overall direction: execution-time resource access should be verified against the actor instead of trusting whatever resource ID was saved in the node config. 👑 The DataTable and credential paths look clearly valid. The DataTable case is especially convincing because the previous check relied on A few things I would like us to address or verify before merge:
Overall, the core issue is valid and worth fixing. These notes are mostly about making the fix robust and avoiding an accidental regression around shared resources. Thanks again for tackling this. 🤞 |
|
Thank you for the thorough review and the feedback! I have addressed all of the brought up points and have pushed the update to the PR. |
|
Thanks for the quick turnaround. The latest commits address the earlier feedback well: the fail-closed One remaining blocker before merge: a few Please forward the actor in these call sites:
A small regression test for one of these paths would also be helpful, since the current tests do not cover this miss. Everything else looks solid. Thanks again for the fast fix. |
The fail-closed resource lookups require an actor; thread it through the dashboard chat, MCP tool, and named MCP server execution paths so credential/DataTable/VectorStore access keeps working. Add a regression test asserting the dashboard chat path forwards the actor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Done! |
mbakgun
left a comment
There was a problem hiding this comment.
Thank you for fixing IDOR ! Such a king 👑 Merged once again 🚀
This tightens workflow execution so nodes use the same ownership/share rules as the API before decrypting or using resources. It passes an actor user into normal, streaming, trigger, portal, sub-workflow, and HITL resume execution paths, then uses that actor when loading credentials, vector stores, and DataTables.
What changed:
Verified with:
uv run ruff format --check app tests/test_agent_node_tools.py tests/test_retry_execution_logs.py tests/test_sse_streaming.py tests/test_workflow_execution_api.pyuv run ruff check app tests/test_agent_node_tools.py tests/test_retry_execution_logs.py tests/test_sse_streaming.py tests/test_workflow_execution_api.py./run_tests.sh