feat: add secret key rotation script and documentation #10978
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 WalkthroughThis PR introduces secret key rotation capabilities to Langflow by adding documentation for the rotation process, a new migration script that handles re-encryption of sensitive database fields, and comprehensive unit tests. The script supports dry-run preview, configuration-based key management, and backup functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant Script as Migration Script
participant KeyMgmt as Key Management
participant DB as Database
participant FileSystem as File System
Script->>KeyMgmt: Read current secret key from<br/>config dir or environment
KeyMgmt-->>Script: Current secret key
Script->>KeyMgmt: Generate new secret key<br/>(if not provided)
KeyMgmt-->>Script: New secret key
Script->>FileSystem: Backup old key with timestamp
FileSystem-->>Script: Backup completed
Script->>DB: Query all encrypted records<br/>(user, variable, folder)
DB-->>Script: Encrypted data
rect rgba(200, 220, 255, 0.3)
Note over Script,DB: Migration Phase (per record)
Script->>Script: Decrypt with old key
Script->>Script: Re-encrypt with new key
Script->>DB: Update record with new ciphertext
end
Script->>FileSystem: Write new secret key to file<br/>(secure permissions: 600)
FileSystem-->>Script: Key saved
Script-->>Script: Generate migration summary<br/>(success/failure counts)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (6 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
enhancement
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (39.42%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #10978 +/- ##
=======================================
Coverage 33.22% 33.22%
=======================================
Files 1391 1391
Lines 65849 65849
Branches 9745 9745
=======================================
+ Hits 21877 21880 +3
+ Misses 42850 42846 -4
- Partials 1122 1123 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
github-actions
bot
added
enhancement
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
scripts/migrate_secret_key.py (1)
49-70: Windows secure permissions fallback should be more prominent.When
pywin32is not installed on Windows, the script only prints a warning but continues, leaving the key file with default permissions (potentially readable by other users). Consider making this a more prominent warning or documenting thatpywin32is required for secure Windows deployments.SECURITY.md (1)
163-172: Specify language for fenced code block.The code block showing CLI options lacks a language identifier. While this is minor, adding a language specifier improves rendering consistency.
-``` +```text python scripts/migrate_secret_key.py --helpsrc/backend/tests/unit/scripts/test_migrate_secret_key.py (3)
19-36: Module loading fixture is well-designed but brittle path calculation.The fixture correctly loads the migration script dynamically, but the hardcoded
parents[5]to reach repo root is fragile. If the test file moves, this will break silently.Consider using a more robust approach to find the repo root:
def migrate_module(): """Load the migrate_secret_key module from scripts directory.""" - # Test file is at: src/backend/tests/unit/scripts/test_migrate_secret_key.py - # Script is at: scripts/migrate_secret_key.py - # Need to go up 5 levels to repo root, then into scripts/ test_file = Path(__file__).resolve() - repo_root = test_file.parents[5] # Goes to langflow repo root + # Find repo root by looking for scripts directory + repo_root = test_file + while repo_root.parent != repo_root: + if (repo_root / "scripts" / "migrate_secret_key.py").exists(): + break + repo_root = repo_root.parent + else: + pytest.skip("Could not find repo root with scripts directory") script_path = repo_root / "scripts" / "migrate_secret_key.py"
255-351: Database migration unit tests cover the core scenarios.The tests validate migration logic for each table type using in-memory SQLite. They manually implement the migration steps rather than calling
migrate()directly, which is appropriate for unit testing individual pieces.Consider adding a test that calls
migrate()end-to-end with a temp database file to verify the full orchestration.
440-454: Unusedmigrate_modulefixture in integration tests.The
migrate_modulefixture is passed but not used in these integration tests (marked with# noqa: ARG002). If it's not needed for setup, consider removing it to reduce confusion.async def test_credential_variable_stored_encrypted( self, - migrate_module, # noqa: ARG002 client: AsyncClient, active_user, # noqa: ARG002 logged_in_headers, ):
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
SECURITY.md(3 hunks)scripts/migrate_secret_key.py(1 hunks)src/backend/tests/unit/scripts/test_migrate_secret_key.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/backend/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
src/backend/**/*.py: Use FastAPI async patterns withawaitfor async operations in component execution methods
Useasyncio.create_task()for background tasks and implement proper cleanup with try/except forasyncio.CancelledError
Usequeue.put_nowait()for non-blocking queue operations andasyncio.wait_for()with timeouts for controlled get operations
Files:
src/backend/tests/unit/scripts/test_migrate_secret_key.py
src/backend/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
src/backend/tests/**/*.py: Place backend unit tests insrc/backend/tests/directory, component tests insrc/backend/tests/unit/components/organized by component subdirectory, and integration tests accessible viamake integration_tests
Use same filename as component with appropriate test prefix/suffix (e.g.,my_component.py→test_my_component.py)
Use theclientfixture (FastAPI Test Client) defined insrc/backend/tests/conftest.pyfor API tests; it provides an asynchttpx.AsyncClientwith automatic in-memory SQLite database and mocked environment variables. Skip client creation by marking test with@pytest.mark.noclient
Inherit from the correctComponentTestBasefamily class located insrc/backend/tests/base.pybased on API access needs:ComponentTestBase(no API),ComponentTestBaseWithClient(needs API), orComponentTestBaseWithoutClient(pure logic). Provide three required fixtures:component_class,default_kwargs, andfile_names_mapping
Create comprehensive unit tests for all new backend components. If unit tests are incomplete, create a corresponding Markdown file documenting manual testing steps and expected outcomes
Test both sync and async code paths, mock external dependencies appropriately, test error handling and edge cases, validate input/output behavior, and test component initialization and configuration
Use@pytest.mark.asynciodecorator for async component tests and ensure async methods are properly awaited
Test background tasks usingasyncio.create_task()and verify completion withasyncio.wait_for()with appropriate timeout constraints
Test queue operations using non-blockingqueue.put_nowait()andasyncio.wait_for(queue.get(), timeout=...)to verify queue processing without blocking
Use@pytest.mark.no_blockbustermarker to skip the blockbuster plugin in specific tests
For database tests that may fail in batch runs, run them sequentially usinguv run pytest src/backend/tests/unit/test_database.pyr...
Files:
src/backend/tests/unit/scripts/test_migrate_secret_key.py
**/test_*.py
📄 CodeRabbit inference engine (Custom checks)
**/test_*.py: Review test files for excessive use of mocks that may indicate poor test design - check if tests have too many mock objects that obscure what's actually being tested
Warn when mocks are used instead of testing real behavior and interactions, and suggest using real objects or test doubles when mocks become excessive
Ensure mocks are used appropriately for external dependencies only, not for core logic
Backend test files should follow the naming convention test_*.py with proper pytest structure
Test files should have descriptive test function names that explain what is being tested
Tests should be organized logically with proper setup and teardown
Consider including edge cases and error conditions for comprehensive test coverage
Verify tests cover both positive and negative scenarios where appropriate
For async functions in backend tests, ensure proper async testing patterns are used with pytest
For API endpoints, verify both success and error response testing
Files:
src/backend/tests/unit/scripts/test_migrate_secret_key.py
🧠 Learnings (6)
📚 Learning: 2025-09-21T09:46:44.276Z
Learnt from: iann0036
Repo: langflow-ai/langflow PR: 9935
File: src/lfx/src/lfx/components/amazon/aws_api_call.py:35-48
Timestamp: 2025-09-21T09:46:44.276Z
Learning: Amazon components in the Langflow bundle consistently require explicit AWS credentials (aws_access_key_id and aws_secret_access_key with required=True) rather than falling back to environment/machine credentials for security reasons in hosted environments.
Applied to files:
SECURITY.md
📚 Learning: 2025-10-29T03:55:50.216Z
Learnt from: ricofurtado
Repo: langflow-ai/langflow PR: 10430
File: src/backend/base/langflow/api/v2/registration.py:0-0
Timestamp: 2025-10-29T03:55:50.216Z
Learning: In the langflow project, user registration endpoints in API v2 (`src/backend/base/langflow/api/v2/registration.py`) are intentionally designed to be unauthenticated to support Desktop application initialization and onboarding flows.
Applied to files:
SECURITY.md
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Use `pytest.mark.api_key_required` and `pytest.mark.no_blockbuster` markers for components that need external APIs; use `MockLanguageModel` from `tests.unit.mock_language_model` for testing without external API keys
Applied to files:
src/backend/tests/unit/scripts/test_migrate_secret_key.py
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Create comprehensive unit tests for all new backend components. If unit tests are incomplete, create a corresponding Markdown file documenting manual testing steps and expected outcomes
Applied to files:
src/backend/tests/unit/scripts/test_migrate_secret_key.py
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Test both sync and async code paths, mock external dependencies appropriately, test error handling and edge cases, validate input/output behavior, and test component initialization and configuration
Applied to files:
src/backend/tests/unit/scripts/test_migrate_secret_key.py
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Place backend unit tests in `src/backend/tests/` directory, component tests in `src/backend/tests/unit/components/` organized by component subdirectory, and integration tests accessible via `make integration_tests`
Applied to files:
src/backend/tests/unit/scripts/test_migrate_secret_key.py
🧬 Code graph analysis (1)
src/backend/tests/unit/scripts/test_migrate_secret_key.py (1)
scripts/migrate_secret_key.py (7)
encrypt_with_key(105-108)decrypt_with_key(99-102)migrate_value(111-117)migrate_auth_settings(120-128)read_secret_key_from_file(73-78)write_secret_key_to_file(81-86)get_config_dir(41-46)
🪛 markdownlint-cli2 (0.18.1)
SECURITY.md
163-163: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
- GitHub Check: Lint Backend / Run Mypy (3.11)
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 2
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 4
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 5
- GitHub Check: Lint Backend / Run Mypy (3.13)
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 1
- GitHub Check: Lint Backend / Run Mypy (3.12)
- GitHub Check: Lint Backend / Run Mypy (3.10)
- GitHub Check: Run Backend Tests / LFX Tests - Python 3.10
- GitHub Check: Run Backend Tests / Integration Tests - Python 3.10
- GitHub Check: Test Docker Images / Test docker images
- GitHub Check: Test Starter Templates
- GitHub Check: Update Component Index
- GitHub Check: Update Starter Projects
- GitHub Check: Run Ruff Check and Format
🔇 Additional comments (12)
scripts/migrate_secret_key.py (4)
179-188: Dry-run implementation is well-structured.The dry-run mode correctly previews what would be migrated without modifying data, while still accessing the database to provide accurate counts. Good implementation.
294-297: Exit code handling is correct.The script appropriately exits with non-zero status on failures in real migration mode while allowing dry-run to exit cleanly even with detected failures, enabling proper CI/CD integration.
300-372: CLI implementation is well-designed.The argument parser provides clear help text, sensible defaults, and useful examples. The
RawDescriptionHelpFormatterpreserves the epilog formatting nicely.
89-96: Verifyensure_valid_keymatches Langflow's auth utils implementation.This function's implementation is functionally equivalent to
langflow.services.auth.utils.ensure_valid_key. Both userandom.seed()with the key string for short keys—a deterministic but non-cryptographically-secure approach—which is required for backward compatibility with existing encrypted keys. The padding logic is identical, though inlined here rather than extracted to a helper function.SECURITY.md (2)
120-150: Comprehensive secret key rotation documentation.The documentation clearly explains what the secret key is used for, provides step-by-step migration instructions, and includes important details about what gets invalidated (sessions). The workflow matches the script implementation.
182-200: Recovery documentation is valuable.Including the recovery procedure with backup restoration steps is excellent. This gives users confidence to perform the migration knowing they have a fallback.
src/backend/tests/unit/scripts/test_migrate_secret_key.py (6)
99-124: Good test coverage for key normalization.Tests cover the key scenarios: long key padding, short key seeding, determinism, and uniqueness. The tests validate that the Fernet constructor accepts the generated keys.
127-158: Encryption/decryption tests are comprehensive.The roundtrip test, wrong-key failure test, and short-key compatibility test provide solid coverage. Checking the Fernet signature prefix on line 142 is a nice touch for format validation.
241-252: Edge case test for empty sensitive fields is valuable.This test correctly verifies that
Noneand empty string values are preserved without attempting decryption, which matches the implementation'sif result.get(field)check.
354-433: File management tests are thorough.Tests cover reading, writing, whitespace handling, missing files, directory creation, custom filenames, and config directory resolution from both defaults and environment variables. Good use of
tempfile.TemporaryDirectoryfor isolation.
507-531: Critical compatibility test validates interoperability.This test is essential - it verifies that the migration script's encryption/decryption is fully compatible with Langflow's
auth_utils. The bidirectional test (Langflow→script and script→Langflow) provides confidence that migrated data will work correctly.
63-96: In-memory SQLite fixture provides good test isolation.The fixture creates the minimal schema needed for testing migration logic. Using in-memory SQLite ensures tests are fast and isolated.
github-actions
bot
added
enhancement
github-actions
bot
added
enhancement
# Conflicts: # src/lfx/src/lfx/_assets/component_index.json
github-actions
bot
added
enhancement
# Conflicts: # src/lfx/src/lfx/_assets/component_index.json
github-actions
bot
added
enhancement
github-actions
bot
added
enhancement
github-actions
bot
added
enhancement
github-actions
bot
added
enhancement
github-actions
bot
added
enhancement
github-actions
bot
added
enhancement
github-actions
bot
added
enhancement
github-actions
bot
added
enhancement
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
Summary
scripts/migrate_secret_key.pyfor rotating theLANGFLOW_SECRET_KEYSECURITY.mdwith secret key rotation documentationWhat the script does
user.store_api_key(Langflow Store API key)variable.value(all encrypted variables)folder.auth_settings(MCP oauth_client_secret, api_key)Usage
Summary by CodeRabbit
Documentation
New Features
✏️ Tip: You can customize this high-level summary in your review settings.