fix(variables): Update decryption logic to only apply to CREDENTIAL_TYPE #11122
Conversation
Update decryption logic to only apply to CREDENTIAL_TYPE variables, ensuring other types are treated as plaintext.
|
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 Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughModified variable service decryption logic to apply decryption selectively. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touchesImportant Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 3 warnings)
✅ Passed checks (3 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
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug where the variable decryption logic was incorrectly attempting to decrypt all variable types, causing InvalidToken errors for non-credential variables that are stored as plaintext. The fix ensures that only CREDENTIAL_TYPE variables undergo decryption, while other types (like GENERIC_TYPE) are returned as-is.
Key Changes:
- Modified
get_variable()to conditionally decrypt onlyCREDENTIAL_TYPEvariables - Updated
get_all()to reverse the type check logic, decrypting credentials and treating other types as plaintext - Removed unused
GENERIC_TYPEimport
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
| value = variable.value | ||
| await logger.adebug(f"Decryption of {variable.type} failed for variable '{variable.name}': {e}.") | ||
| value = None # Don't expose potentially corrupted credential data |
There was a problem hiding this comment.
Setting value = None when credential decryption fails could break consumers expecting a string value. Consider raising an exception or logging the error and returning a clear error indicator instead of silently returning None, which may cause downstream null reference issues.
| value = None # Don't expose potentially corrupted credential data | |
| value = "<DECRYPTION_FAILED>" # Don't expose potentially corrupted credential data |
| f"Decryption of {variable.type} failed for variable '{variable.name}': {e}. Assuming plaintext." | ||
| ) | ||
| value = variable.value | ||
| await logger.adebug(f"Decryption of {variable.type} failed for variable '{variable.name}': {e}.") |
There was a problem hiding this comment.
The error message for failed credential decryption should provide more actionable guidance. Consider including information about potential causes (corrupted data, key rotation) and next steps for the user.
| await logger.adebug(f"Decryption of {variable.type} failed for variable '{variable.name}': {e}.") | |
| await logger.adebug( | |
| "Failed to decrypt credential variable '%s' of type '%s': %s. " | |
| "This can happen if the encryption key or settings have changed, or if the stored data " | |
| "is corrupted. Ask the user to re-enter this credential or verify the encryption/key " | |
| "configuration before retrying.", | |
| variable.name, | |
| variable.type, | |
| e, | |
| ) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/backend/base/langflow/services/variable/service.py (1)
14-14: Remove unused import to fix pipeline failure.The
GENERIC_TYPEimport is flagged as unused by the Ruff style checker, causing the pipeline to fail.🔎 Proposed fix
-from langflow.services.variable.constants import CREDENTIAL_TYPE, GENERIC_TYPE +from langflow.services.variable.constants import CREDENTIAL_TYPE
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/backend/base/langflow/services/variable/service.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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/base/langflow/services/variable/service.py
🧬 Code graph analysis (1)
src/backend/base/langflow/services/variable/service.py (2)
src/backend/base/langflow/services/auth/utils.py (1)
decrypt_api_key(567-593)src/backend/base/langflow/services/database/models/variable/model.py (2)
VariableRead(52-64)Variable(25-45)
🪛 GitHub Actions: Ruff Style Check
src/backend/base/langflow/services/variable/service.py
[error] 14-14: F401 'GENERIC_TYPE' imported but unused.
⏰ 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). (19)
- 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 1
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 4
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
- GitHub Check: Test Docker Images / Test docker images
- GitHub Check: Lint Backend / Run Mypy (3.13)
- 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 / Integration Tests - Python 3.10
- GitHub Check: Lint Backend / Run Mypy (3.12)
- GitHub Check: Run Backend Tests / LFX Tests - Python 3.10
- GitHub Check: Test Starter Templates
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (python)
- GitHub Check: Update Starter Projects
- GitHub Check: Update Component Index
- GitHub Check: Optimize new Python code in this PR
🔇 Additional comments (1)
src/backend/base/langflow/services/variable/service.py (1)
187-202: LGTM! Secure error handling and correct type-based decryption.The updated logic correctly:
- Decrypts only
CREDENTIAL_TYPEvariables- Sets
valuetoNoneon decryption failure to prevent exposing potentially corrupted credential data- Returns plaintext for non-credential types
Minor: The comment on line 187 mentions "Generic variables" specifically, but the logic handles all non-
CREDENTIAL_TYPEvariables as plaintext, which is the correct behavior.
| # Only decrypt CREDENTIAL_TYPE variables - others are stored as plaintext | ||
| if variable.type == CREDENTIAL_TYPE: | ||
| return auth_utils.decrypt_api_key(variable.value, settings_service=self.settings_service) | ||
| return variable.value |
There was a problem hiding this comment.
Add error handling for decryption failures to match get_all.
The get_all method (lines 193-199) includes a try-catch block to handle decryption failures and logs the error, but get_variable does not. If decrypt_api_key raises an exception (e.g., due to corrupted data or key mismatch), it will propagate unhandled to the caller, leading to inconsistent error behavior across methods.
🔎 Proposed fix to add consistent error handling
# Only decrypt CREDENTIAL_TYPE variables - others are stored as plaintext
if variable.type == CREDENTIAL_TYPE:
- return auth_utils.decrypt_api_key(variable.value, settings_service=self.settings_service)
+ try:
+ return auth_utils.decrypt_api_key(variable.value, settings_service=self.settings_service)
+ except Exception as e: # noqa: BLE001
+ await logger.adebug(
+ f"Decryption of {variable.type} failed for variable '{variable.name}': {e}."
+ )
+ msg = f"Failed to decrypt variable {name}"
+ raise ValueError(msg) from e
return variable.valueCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/backend/base/langflow/services/variable/service.py around lines 179 to
182, get_variable currently calls auth_utils.decrypt_api_key directly for
CREDENTIAL_TYPE variables without error handling; wrap the decrypt_api_key call
in a try/except that mirrors get_all: catch exceptions from decrypt_api_key, log
the error with the same logger and message format used in get_all (include
exception details), and return None (or a safe fallback) when decryption fails
so the method's behavior matches get_all's error-handling pattern.
1 participant
Update decryption logic to only apply to CREDENTIAL_TYPE variables, ensuring other types are treated as plaintext.
Bug Fix: Langflow Variable Decryption Mismatch (InvalidToken)
Date: 2025-12-21
Status: Resolved
1. Issue Description
InvalidTokenerrors:Decryption using UTF-8 encoded API key failed. Error: . Retrying decryption using the raw string input.Decryption of Generic failed for variable ... Assuming plaintext.POST /api/v1/custom_component/update HTTP/1.1" 400errors during component updates.Invalid base64-encoded string: number of data characters (21) cannot be 1 more than a multiple of 4.update_params_with_load_from_db_fieldswas called, triggeringget_variableorget_allinDatabaseVariableService. This happens frequently when the UI tries to load variable values for components.2. Root Cause Analysis
DatabaseVariableService.get_variableandget_allmethods inservice.py.get_variablewas unconditionally callingauth_utils.decrypt_api_key(variable.value)regardless of the variable type.get_allwas attempting to decryptGENERIC_TYPEvariables (labeled as "attempt to decrypt") but failing because they were plaintext.create_variableandupdate_variableonly encrypt variables iftype == CREDENTIAL_TYPE.get_variableandget_alltried to decrypt everything.GENERIC_TYPEvariable (stored as plaintext) was passed to the Fernet decryptor, it raisedInvalidTokenbecause the plaintext string was not a valid Fernet token (base64 encoded).3. The Fix
src/backend/base/langflow/services/variable/service.py.get_variableto checkif variable.type == CREDENTIAL_TYPEbefore decrypting.get_allto checkif variable.type == CREDENTIAL_TYPEbefore decrypting, and treat others as plaintext.4. Verification
InvalidTokenandDecryption failederror logs disappeared.custom_component/updateshould now be resolved as the variables are correctly returned as plaintext without throwing exceptions.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.