fix: Scope flow access to user owning flow #10222
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces a new security module for flow ownership/public access checks and integrates it across core graph building, chat, file endpoints, and helper utilities. Multiple functions now delegate to centralized ownership/public retrieval. Tests are added for ownership validation and public access, with some endpoint test scaffolding. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant API as API Endpoint (e.g., Files/Build)
participant Dep as Depends(get_flow)
participant Sec as Security Helpers
participant DB as DB (AsyncSession)
U->>API: Request (flow_id or endpoint_name)
API->>Dep: Resolve Flow dependency
Dep->>Sec: get_flow_with_ownership(session, id/name, user_id)
Sec->>DB: Query Flow (by UUID or endpoint_name + owner)
DB-->>Sec: Flow or None
alt Found and owned
Sec-->>Dep: Flow
Dep-->>API: Flow
API-->>U: Proceed with operation
else Not found / not owned
Sec-->>Dep: HTTP 404
Dep-->>API: Propagate error
API-->>U: 404 Not Found
end
sequenceDiagram
autonumber
actor Any as Any User/Anon
participant API as Public Access (e.g., Resolve Owner by Flow)
participant Sec as Security Helpers
participant DB as DB
Any->>API: Request with flow id/name
API->>Sec: get_public_flow_by_name_or_id(session, id/name)
Sec->>DB: Query Flow where access_type = PUBLIC
DB-->>Sec: Flow or None
alt Public flow found
Sec-->>API: Flow
API-->>Any: Public data (e.g., owner info)
else Not public or missing
Sec-->>API: HTTP 404
API-->>Any: 404 Not Found
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/backend/tests/unit/security/test_flow_access_security.py (1)
74-77: Duplicate User instantiation issue across multiple tests.The same User instantiation pattern (without
hashed_password) appears in multiple test methods at lines 74-77, 93-96, and 138. If the User model requireshashed_password, all these tests will fail.See the earlier comment on lines 34-37 for the verification script and fix.
Also applies to: 93-96, 138-138
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/backend/base/langflow/api/security.py(1 hunks)src/backend/base/langflow/api/utils/core.py(2 hunks)src/backend/base/langflow/api/v1/chat.py(1 hunks)src/backend/base/langflow/api/v1/files.py(4 hunks)src/backend/base/langflow/helpers/flow.py(3 hunks)src/backend/base/langflow/helpers/user.py(1 hunks)src/backend/tests/integration/test_security_integration.py(1 hunks)src/backend/tests/unit/security/test_flow_access_security.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
{src/backend/**/*.py,tests/**/*.py,Makefile}
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
{src/backend/**/*.py,tests/**/*.py,Makefile}: Run make format_backend to format Python code before linting or committing changes
Run make lint to perform linting checks on backend Python code
Files:
src/backend/base/langflow/api/v1/chat.pysrc/backend/tests/integration/test_security_integration.pysrc/backend/base/langflow/helpers/flow.pysrc/backend/base/langflow/helpers/user.pysrc/backend/tests/unit/security/test_flow_access_security.pysrc/backend/base/langflow/api/utils/core.pysrc/backend/base/langflow/api/security.pysrc/backend/base/langflow/api/v1/files.py
src/backend/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
src/backend/tests/**/*.py: Unit tests for backend code must be located in the 'src/backend/tests/' directory, with component tests organized by component subdirectory under 'src/backend/tests/unit/components/'.
Test files should use the same filename as the component under test, with an appropriate test prefix or suffix (e.g., 'my_component.py' → 'test_my_component.py').
Use the 'client' fixture (an async httpx.AsyncClient) for API tests in backend Python tests, as defined in 'src/backend/tests/conftest.py'.
When writing component tests, inherit from the appropriate base class in 'src/backend/tests/base.py' (ComponentTestBase, ComponentTestBaseWithClient, or ComponentTestBaseWithoutClient) and provide the required fixtures: 'component_class', 'default_kwargs', and 'file_names_mapping'.
Each test in backend Python test files should have a clear docstring explaining its purpose, and complex setups or mocks should be well-commented.
Test both sync and async code paths in backend Python tests, using '@pytest.mark.asyncio' for async tests.
Mock external dependencies appropriately in backend Python tests to isolate unit tests from external services.
Test error handling and edge cases in backend Python tests, including using 'pytest.raises' and asserting error messages.
Validate input/output behavior and test component initialization and configuration in backend Python tests.
Use the 'no_blockbuster' pytest marker to skip the blockbuster plugin in tests when necessary.
Be aware of ContextVar propagation in async tests; test both direct event loop execution and 'asyncio.to_thread' scenarios to ensure proper context isolation.
Test error handling by mocking internal functions using monkeypatch in backend Python tests.
Test resource cleanup in backend Python tests by using fixtures that ensure proper initialization and cleanup of resources.
Test timeout and performance constraints in backend Python tests using 'asyncio.wait_for' and timing assertions.
Test Langflow's Messag...
Files:
src/backend/tests/integration/test_security_integration.pysrc/backend/tests/unit/security/test_flow_access_security.py
**/{test_*.py,*.test.ts,*.test.tsx}
📄 CodeRabbit inference engine (coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt)
**/{test_*.py,*.test.ts,*.test.tsx}: Review tests for excessive numbers of mocks that obscure the behavior under test
Warn when mocks replace real behavior/interactions that should be exercised by tests
Suggest using real objects or lightweight test doubles when mocks become excessive
Ensure mocks are reserved for external dependencies, not core domain logic
Recommend integration tests when unit tests rely heavily on mocks
Check that test files follow project naming conventions (backend: test_*.py; frontend: *.test.ts/tsx)
Verify tests actually exercise the new functionality (avoid placeholder tests)
Test files should use descriptive test names that explain the behavior under test
Organize tests logically with proper setup and teardown
Include edge cases and error conditions for comprehensive coverage
Cover both positive and negative scenarios where appropriate
Tests should cover the main functionality being implemented
Avoid smoke-only tests; assert concrete behaviors and outcomes
Follow project testing tools: pytest for backend, Playwright for frontend
For API endpoints, include tests for both success and error responses
Files:
src/backend/tests/integration/test_security_integration.pysrc/backend/tests/unit/security/test_flow_access_security.py
**/test_*.py
📄 CodeRabbit inference engine (coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt)
**/test_*.py: Backend tests must use pytest structure with files named test_*.py
For async Python functions, use proper async testing patterns with pytest
Files:
src/backend/tests/integration/test_security_integration.pysrc/backend/tests/unit/security/test_flow_access_security.py
src/backend/tests/unit/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
Test component integration within flows using create_flow, build_flow, and get_build_events utilities
Files:
src/backend/tests/unit/security/test_flow_access_security.py
🧠 Learnings (1)
📚 Learning: 2025-07-21T14:16:14.125Z
Learnt from: CR
PR: langflow-ai/langflow#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-21T14:16:14.125Z
Learning: Applies to src/backend/tests/**/*.py : Test Langflow's REST API endpoints in backend Python tests using the async client fixture and asserting response codes and payloads.
Applied to files:
src/backend/tests/unit/security/test_flow_access_security.py
🧬 Code graph analysis (8)
src/backend/base/langflow/api/v1/chat.py (2)
src/backend/base/langflow/api/security.py (1)
get_flow_with_ownership(11-37)src/backend/base/langflow/services/deps.py (1)
session_scope(151-173)
src/backend/tests/integration/test_security_integration.py (1)
src/backend/base/langflow/api/security.py (3)
get_flow_with_ownership(11-37)get_flow_with_ownership_by_name_or_id(40-71)get_public_flow_by_name_or_id(74-106)
src/backend/base/langflow/helpers/flow.py (3)
src/backend/base/langflow/api/security.py (2)
get_flow_with_ownership(11-37)get_flow_with_ownership_by_name_or_id(40-71)src/backend/base/langflow/services/database/models/flow/model.py (1)
FlowRead(221-225)src/backend/base/langflow/services/deps.py (1)
session_scope(151-173)
src/backend/base/langflow/helpers/user.py (3)
src/backend/base/langflow/services/database/models/user/model.py (1)
UserRead(62-72)src/backend/base/langflow/api/security.py (2)
get_flow_with_ownership_by_name_or_id(40-71)get_public_flow_by_name_or_id(74-106)src/backend/base/langflow/services/deps.py (1)
session_scope(151-173)
src/backend/tests/unit/security/test_flow_access_security.py (2)
src/backend/base/langflow/api/security.py (3)
get_flow_with_ownership(11-37)get_flow_with_ownership_by_name_or_id(40-71)get_public_flow_by_name_or_id(74-106)src/backend/base/langflow/services/database/models/flow/model.py (2)
AccessTypeEnum(27-29)Flow(186-212)
src/backend/base/langflow/api/utils/core.py (3)
src/backend/base/langflow/services/deps.py (1)
session_scope(151-173)src/backend/base/langflow/api/security.py (1)
get_flow_with_ownership(11-37)src/backend/base/langflow/services/database/models/flow/model.py (1)
Flow(186-212)
src/backend/base/langflow/api/security.py (2)
src/backend/tests/conftest.py (1)
flow(544-560)src/backend/base/langflow/services/database/models/flow/model.py (2)
Flow(186-212)AccessTypeEnum(27-29)
src/backend/base/langflow/api/v1/files.py (4)
src/backend/base/langflow/api/security.py (1)
get_flow_with_ownership(11-37)src/backend/base/langflow/services/database/models/flow/model.py (1)
Flow(186-212)src/backend/base/langflow/services/storage/service.py (1)
StorageService(16-52)src/backend/base/langflow/services/deps.py (1)
get_storage_service(88-96)
🔇 Additional comments (5)
src/backend/base/langflow/helpers/user.py (2)
30-35: Verify exception handling doesn't mask non-ownership errors.The empty
passblock silently swallows allHTTPExceptioninstances raised byget_flow_with_ownership_by_name_or_id. While this enables the intended fallback to public flow access, it may also mask unexpected errors (e.g., database connection issues, malformed flow data) that should bubble up.Consider narrowing the exception handling to only catch ownership-related 404s, or add logging to track when the fallback path is taken:
if requesting_user_id: try: flow = await get_flow_with_ownership_by_name_or_id(session, flow_id_or_name, requesting_user_id) - except HTTPException: + except HTTPException as e: + if e.status_code != 404: + raise # If ownership fails, try public flow access - passAlternatively, verify through testing or code inspection that
get_flow_with_ownership_by_name_or_idonly raises 404 HTTPExceptions for ownership failures.
9-45: Ownership validation logic is well-structured.The two-step flow resolution (ownership-validated → public fallback) appropriately enforces security while maintaining backward compatibility for public flows. The function-scoped import at line 24 is an acceptable pattern to avoid circular dependencies.
src/backend/tests/unit/security/test_flow_access_security.py (1)
23-128: Comprehensive security test coverage in TestFlowAccessSecurity.The implemented tests in
TestFlowAccessSecuritythoroughly validate:
- Owner access control
- Cross-account blocking
- Nonexistent flow handling
- UUID and endpoint name resolution
- Public vs private flow access
This provides strong coverage of the core security functions.
src/backend/base/langflow/api/v1/files.py (2)
27-32: Centralized ownership validation improves security posture.The
get_flowdependency consolidates flow ownership checks in a single location, eliminating duplicate validation logic and reducing the risk of bypassing security checks. This follows the DRY principle and makes future security updates easier to maintain.
35-65: Consistent security refactoring across all file endpoints.All file operation endpoints (
upload_file,download_file,download_image,list_files,delete_file) have been successfully refactored to use the centralizedget_flowdependency. The ownership validation is now enforced uniformly, preventing potential cross-account file access vulnerabilities.Also applies to: 68-96, 99-121, 165-175, 178-189
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
src/backend/tests/unit/security/test_flow_access_security.py (1)
153-165: Implement or remove placeholder tests.Similar to the file endpoint tests, these are placeholder tests that don't verify any behavior. As per coding guidelines, tests should cover the main functionality being implemented, not just provide empty scaffolding.
🧹 Nitpick comments (2)
src/backend/base/langflow/api/v1/files.py (1)
23-32: Move import to top of file per PEP 8.The import of
get_flow_with_ownershipshould be placed with the other imports at the top of the file to follow Python style conventions (PEP 8).Apply this diff to move the import:
from langflow.api.utils import CurrentActiveUser, DbSession from langflow.api.v1.schemas import UploadFileResponse +from langflow.api.security import get_flow_with_ownership from langflow.services.database.models.flow.model import Flow from langflow.services.deps import get_settings_service, get_storage_service from langflow.services.storage.service import StorageService router = APIRouter(tags=["Files"], prefix="/files") -from langflow.api.security import get_flow_with_ownership - # Secure dependency that validates flow ownership async def get_flow(src/backend/tests/unit/security/test_flow_access_security.py (1)
1-1: Add init.py to security test directory.The
src/backend/tests/unit/security/directory is missing an__init__.pyfile, making it an implicit namespace package. Add an empty__init__.pyfile to properly structure the package.Create an empty file:
# src/backend/tests/unit/security/__init__.py
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/backend/base/langflow/api/security.py(1 hunks)src/backend/base/langflow/api/utils/core.py(2 hunks)src/backend/base/langflow/api/v1/chat.py(1 hunks)src/backend/base/langflow/api/v1/files.py(4 hunks)src/backend/base/langflow/helpers/flow.py(3 hunks)src/backend/base/langflow/helpers/user.py(1 hunks)src/backend/tests/integration/test_security_integration.py(1 hunks)src/backend/tests/unit/security/test_flow_access_security.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
{src/backend/**/*.py,tests/**/*.py,Makefile}
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
{src/backend/**/*.py,tests/**/*.py,Makefile}: Run make format_backend to format Python code before linting or committing changes
Run make lint to perform linting checks on backend Python code
Files:
src/backend/base/langflow/api/v1/chat.pysrc/backend/tests/integration/test_security_integration.pysrc/backend/base/langflow/api/utils/core.pysrc/backend/base/langflow/api/security.pysrc/backend/base/langflow/helpers/user.pysrc/backend/base/langflow/helpers/flow.pysrc/backend/base/langflow/api/v1/files.pysrc/backend/tests/unit/security/test_flow_access_security.py
src/backend/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
src/backend/tests/**/*.py: Unit tests for backend code must be located in the 'src/backend/tests/' directory, with component tests organized by component subdirectory under 'src/backend/tests/unit/components/'.
Test files should use the same filename as the component under test, with an appropriate test prefix or suffix (e.g., 'my_component.py' → 'test_my_component.py').
Use the 'client' fixture (an async httpx.AsyncClient) for API tests in backend Python tests, as defined in 'src/backend/tests/conftest.py'.
When writing component tests, inherit from the appropriate base class in 'src/backend/tests/base.py' (ComponentTestBase, ComponentTestBaseWithClient, or ComponentTestBaseWithoutClient) and provide the required fixtures: 'component_class', 'default_kwargs', and 'file_names_mapping'.
Each test in backend Python test files should have a clear docstring explaining its purpose, and complex setups or mocks should be well-commented.
Test both sync and async code paths in backend Python tests, using '@pytest.mark.asyncio' for async tests.
Mock external dependencies appropriately in backend Python tests to isolate unit tests from external services.
Test error handling and edge cases in backend Python tests, including using 'pytest.raises' and asserting error messages.
Validate input/output behavior and test component initialization and configuration in backend Python tests.
Use the 'no_blockbuster' pytest marker to skip the blockbuster plugin in tests when necessary.
Be aware of ContextVar propagation in async tests; test both direct event loop execution and 'asyncio.to_thread' scenarios to ensure proper context isolation.
Test error handling by mocking internal functions using monkeypatch in backend Python tests.
Test resource cleanup in backend Python tests by using fixtures that ensure proper initialization and cleanup of resources.
Test timeout and performance constraints in backend Python tests using 'asyncio.wait_for' and timing assertions.
Test Langflow's Messag...
Files:
src/backend/tests/integration/test_security_integration.pysrc/backend/tests/unit/security/test_flow_access_security.py
**/{test_*.py,*.test.ts,*.test.tsx}
📄 CodeRabbit inference engine (coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt)
**/{test_*.py,*.test.ts,*.test.tsx}: Review tests for excessive numbers of mocks that obscure the behavior under test
Warn when mocks replace real behavior/interactions that should be exercised by tests
Suggest using real objects or lightweight test doubles when mocks become excessive
Ensure mocks are reserved for external dependencies, not core domain logic
Recommend integration tests when unit tests rely heavily on mocks
Check that test files follow project naming conventions (backend: test_*.py; frontend: *.test.ts/tsx)
Verify tests actually exercise the new functionality (avoid placeholder tests)
Test files should use descriptive test names that explain the behavior under test
Organize tests logically with proper setup and teardown
Include edge cases and error conditions for comprehensive coverage
Cover both positive and negative scenarios where appropriate
Tests should cover the main functionality being implemented
Avoid smoke-only tests; assert concrete behaviors and outcomes
Follow project testing tools: pytest for backend, Playwright for frontend
For API endpoints, include tests for both success and error responses
Files:
src/backend/tests/integration/test_security_integration.pysrc/backend/tests/unit/security/test_flow_access_security.py
**/test_*.py
📄 CodeRabbit inference engine (coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt)
**/test_*.py: Backend tests must use pytest structure with files named test_*.py
For async Python functions, use proper async testing patterns with pytest
Files:
src/backend/tests/integration/test_security_integration.pysrc/backend/tests/unit/security/test_flow_access_security.py
src/backend/tests/unit/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
Test component integration within flows using create_flow, build_flow, and get_build_events utilities
Files:
src/backend/tests/unit/security/test_flow_access_security.py
🧠 Learnings (1)
📚 Learning: 2025-07-21T14:16:14.125Z
Learnt from: CR
PR: langflow-ai/langflow#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-21T14:16:14.125Z
Learning: Applies to src/backend/tests/**/*.py : Test Langflow's REST API endpoints in backend Python tests using the async client fixture and asserting response codes and payloads.
Applied to files:
src/backend/tests/unit/security/test_flow_access_security.py
🧬 Code graph analysis (8)
src/backend/base/langflow/api/v1/chat.py (3)
src/backend/base/langflow/api/security.py (1)
get_flow_with_ownership(11-37)src/backend/base/langflow/services/deps.py (1)
session_scope(151-173)src/backend/tests/conftest.py (1)
flow(544-560)
src/backend/tests/integration/test_security_integration.py (1)
src/backend/base/langflow/api/security.py (3)
get_flow_with_ownership(11-37)get_flow_with_ownership_by_name_or_id(40-71)get_public_flow_by_name_or_id(74-106)
src/backend/base/langflow/api/utils/core.py (4)
src/backend/base/langflow/services/deps.py (1)
session_scope(151-173)src/backend/base/langflow/api/security.py (1)
get_flow_with_ownership(11-37)src/backend/tests/conftest.py (1)
flow(544-560)src/backend/base/langflow/services/database/models/flow/model.py (1)
Flow(186-212)
src/backend/base/langflow/api/security.py (2)
src/backend/tests/conftest.py (1)
flow(544-560)src/backend/base/langflow/services/database/models/flow/model.py (2)
Flow(186-212)AccessTypeEnum(27-29)
src/backend/base/langflow/helpers/user.py (3)
src/backend/base/langflow/services/database/models/user/model.py (1)
UserRead(62-72)src/backend/base/langflow/api/security.py (2)
get_flow_with_ownership_by_name_or_id(40-71)get_public_flow_by_name_or_id(74-106)src/backend/base/langflow/services/deps.py (1)
session_scope(151-173)
src/backend/base/langflow/helpers/flow.py (3)
src/backend/base/langflow/api/security.py (2)
get_flow_with_ownership(11-37)get_flow_with_ownership_by_name_or_id(40-71)src/backend/base/langflow/services/database/models/flow/model.py (1)
FlowRead(221-225)src/backend/base/langflow/services/deps.py (1)
session_scope(151-173)
src/backend/base/langflow/api/v1/files.py (4)
src/backend/base/langflow/api/security.py (1)
get_flow_with_ownership(11-37)src/backend/base/langflow/services/database/models/flow/model.py (1)
Flow(186-212)src/backend/base/langflow/services/storage/service.py (1)
StorageService(16-52)src/backend/base/langflow/services/deps.py (1)
get_storage_service(88-96)
src/backend/tests/unit/security/test_flow_access_security.py (3)
src/backend/base/langflow/api/security.py (3)
get_flow_with_ownership(11-37)get_flow_with_ownership_by_name_or_id(40-71)get_public_flow_by_name_or_id(74-106)src/backend/tests/conftest.py (1)
flow(544-560)src/backend/base/langflow/services/database/models/flow/model.py (2)
AccessTypeEnum(27-29)Flow(186-212)
🪛 GitHub Actions: Ruff Style Check
src/backend/base/langflow/api/security.py
[error] 70-70: B904 Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling.
🪛 GitHub Check: Ruff Style Check (3.13)
src/backend/base/langflow/api/v1/chat.py
[failure] 170-170: Ruff (F841)
src/backend/base/langflow/api/v1/chat.py:170:9: F841 Local variable flow is assigned to but never used
src/backend/base/langflow/api/utils/core.py
[failure] 224-224: Ruff (F821)
src/backend/base/langflow/api/utils/core.py:224:64: F821 Undefined name UUID
src/backend/base/langflow/api/security.py
[failure] 70-70: Ruff (B904)
src/backend/base/langflow/api/security.py:70:13: B904 Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
src/backend/base/langflow/helpers/user.py
[failure] 31-35: Ruff (SIM105)
src/backend/base/langflow/helpers/user.py:31:13: SIM105 Use contextlib.suppress(HTTPException) instead of try-except-pass
src/backend/base/langflow/api/v1/files.py
[failure] 23-23: Ruff (E402)
src/backend/base/langflow/api/v1/files.py:23:1: E402 Module level import not at top of file
src/backend/tests/unit/security/test_flow_access_security.py
[failure] 90-90: Ruff (ARG002)
src/backend/tests/unit/security/test_flow_access_security.py:90:66: ARG002 Unused method argument: created_user
[failure] 71-71: Ruff (ARG002)
src/backend/tests/unit/security/test_flow_access_security.py:71:83: ARG002 Unused method argument: created_user
[failure] 31-31: Ruff (ARG002)
src/backend/tests/unit/security/test_flow_access_security.py:31:73: ARG002 Unused method argument: created_user
[failure] 1-1: Ruff (INP001)
src/backend/tests/unit/security/test_flow_access_security.py:1:1: INP001 File src/backend/tests/unit/security/test_flow_access_security.py is part of an implicit namespace package. Add an __init__.py.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: Validate PR
- GitHub Check: Lint Backend / Run Mypy (3.13)
- GitHub Check: Lint Backend / Run Mypy (3.12)
- GitHub Check: Test Docker Images / Test docker images
- GitHub Check: Lint Backend / Run Mypy (3.11)
- GitHub Check: Run Frontend Tests / Determine Test Suites and Shard Distribution
- GitHub Check: Lint Backend / Run Mypy (3.10)
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 4
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 1
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 5
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 2
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
- GitHub Check: Run Backend Tests / Integration Tests - Python 3.10
- GitHub Check: Test Starter Templates
- GitHub Check: Optimize new Python code in this PR
- GitHub Check: Update Starter Projects
🔇 Additional comments (7)
src/backend/base/langflow/api/utils/core.py (1)
153-178: ImportUUID(currently undefined).Both
_get_flow_nameandbuild_graph_from_db_no_cachereferenceUUID, but the module only importsimport uuid. At runtime this raisesNameError. Addfrom uuid import UUIDat the top or useuuid.UUID.-from __future__ import annotations - -import uuid +from __future__ import annotations + +import uuid +from uuid import UUIDLikely an incorrect or invalid review comment.
src/backend/base/langflow/api/security.py (2)
11-37: LGTM!The ownership validation logic is correct and secure. By combining both
flow_idanduser_idin the WHERE clause, this function ensures that flows can only be accessed by their owners, effectively preventing cross-account access vulnerabilities.
74-106: LGTM!The public flow retrieval logic is correct. By filtering on
access_type == AccessTypeEnum.PUBLIC, this function ensures that only explicitly public flows can be accessed without ownership validation. The warning in the docstring appropriately highlights the security considerations.src/backend/base/langflow/api/v1/files.py (3)
35-65: LGTM!The refactor to use
Flowdependency with ownership validation is excellent. By moving the security check to theget_flowdependency, you've centralized the ownership validation and made it impossible to bypass. The comment at line 54 correctly notes that the security check is handled by the dependency.
68-96: LGTM!The refactor correctly uses the
Flowdependency to enforce ownership validation before allowing file downloads.
99-121: LGTM!The refactor correctly uses the
Flowdependency to enforce ownership validation before allowing image downloads.src/backend/tests/unit/security/test_flow_access_security.py (1)
20-129: LGTM!The test suite comprehensively validates the ownership and access control logic. Tests cover:
- Valid owner access
- Cross-account blocking
- Nonexistent flow handling
- UUID and endpoint name resolution
- Public vs private flow access
Each test is well-documented and properly uses pytest patterns.
Note on unused argument warnings: The static analysis warnings about unused
created_userarguments in some tests are false positives. These fixture arguments are necessary to ensure the user exists in the database and to establish the proper test state, even if not directly referenced in the test body.
github-actions
bot
added
bug
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
|
|
Please follow up this PR with the fix: #10306 |
2 participants
This pull request introduces comprehensive security improvements around flow ownership and access control throughout the backend. The main focus is to ensure that users can only access, modify, or interact with flows that they own, effectively preventing unauthorized or cross-account access. A new set of utility functions for secure flow retrieval and validation has been added, and these are now used consistently across API endpoints and internal helpers.
Security and Access Control Enhancements:
langflow/api/security.pycontaining async helper functions (get_flow_with_ownership,get_flow_with_ownership_by_name_or_id,get_public_flow_by_name_or_id) that enforce strict flow ownership checks and public access validation. These functions are now the standard for flow retrieval throughout the backend._get_flow_name,build_graph_from_db_no_cache) to use the new secure flow retrieval helpers, ensuring that any access to flow data is validated against the requesting user's ownership or explicit public status. [1] [2]api/v1/files.pyto use a new internal dependency (_get_flow_with_validation) that wraps the secure ownership check, replacing previous manual checks. [1] [2] [3] [4] [5] [6] [7]API Endpoint Security:
retrieve_vertices_order,build_flow,build_vertex) by invokingget_flow_with_ownershipbefore any operation that accesses or builds on a flow. This ensures only the owner can trigger these actions. [1] [2] [3] [4] [5] [6] [7]Internal Helper Refactoring:
load_flowhelper to use the secure ownership validation function, replacing direct database access with explicit security checks. This ensures all internal graph loading is protected against unauthorized access. [1] [2]These changes collectively provide a robust and consistent security layer for flow access across the backend, reducing the risk of unauthorized data exposure or manipulation.
References:
[1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19]