fix: redact sensitive information from log output#12271
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughChanges reduce sensitive data exposure in logging across multiple backend modules by replacing full object/dictionary logging with redacted messages or key-only logs, and masking credentials (API keys, passwords, JWT tokens) to display only the last 4 characters. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (45.53%) 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 @@
## release-1.9.0 #12271 +/- ##
=================================================
+ Coverage 48.86% 48.91% +0.04%
=================================================
Files 1897 1896 -1
Lines 167656 167624 -32
Branches 23193 23120 -73
=================================================
+ Hits 81928 81987 +59
+ Misses 84817 84727 -90
+ Partials 911 910 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/backend/tests/locust/langflow_setup_test.py (1)
407-418:⚠️ Potential issue | 🟡 MinorCredentials file still contains full sensitive values.
The
save_credentialsfunction writes the full unmasked password, access_token, and api_key to a JSON file. While this may be intentional for the load testing use case, it's inconsistent with the redaction approach inprint_setup_results. Consider adding a warning when saving credentials to file, or document this explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/tests/locust/langflow_setup_test.py` around lines 407 - 418, The save_credentials function is currently persisting full sensitive values (password, access_token, api_key) while print_setup_results redacts them; update save_credentials to either redact these fields before writing or emit a clear warning/log that full credentials are being written so reviewers/users are aware. Concretely, in save_credentials replace or duplicate sensitive fields (password, access_token, api_key) with masked versions (e.g., keep last 4 chars) before serializing, and/or add a processLogger.warning call (or raise a flag) in the same function to document that unredacted credentials will be saved; reference save_credentials and print_setup_results so the change is consistent with the redaction behavior.
🧹 Nitpick comments (2)
src/backend/tests/locust/langflow_setup_test.py (1)
360-362: Consider simplifying the conditional print statement.The inline conditional expression spanning multiple lines is harder to read. Consider refactoring for clarity.
Proposed refactor
- print(f"JWT Token: ***{setup_state['access_token'][-4:]}") if setup_state["access_token"] else print( - "JWT Token: N/A" - ) + if setup_state["access_token"]: + print(f"JWT Token: ***{setup_state['access_token'][-4:]}") + else: + print("JWT Token: N/A")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/tests/locust/langflow_setup_test.py` around lines 360 - 362, Replace the inline conditional print with a simple if/else for readability: check setup_state["access_token"] and if present print the masked token using setup_state['access_token'][-4:], otherwise print "JWT Token: N/A"; update the code around the existing print statement that references setup_state and access_token (the block currently using the inline conditional) to use this clearer if/else structure.src/backend/base/langflow/api/v1/openai_responses.py (1)
620-622: Redundant log statement.Line 622 logs
Variables dict keys: {list(variables.keys())}which duplicates the information already logged on line 621 (Extracted global variables from headers: {list(variables.keys())}). Consider removing the redundant log.Proposed fix
await logger.adebug(f"All headers received: {list(http_request.headers.keys())}") await logger.adebug(f"Extracted global variables from headers: {list(variables.keys())}") - await logger.adebug(f"Variables dict keys: {list(variables.keys())}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/base/langflow/api/v1/openai_responses.py` around lines 620 - 622, Remove the redundant debug log that repeats the same keys already logged: delete the second/third call to logger.adebug that prints list(variables.keys()) so only one meaningful message remains (e.g., keep "Extracted global variables from headers: {list(variables.keys())}"); locate the three consecutive await logger.adebug calls (the ones referencing http_request.headers and variables) and remove the duplicate "Variables dict keys" logger call to avoid duplicated output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/backend/base/langflow/api/utils/mcp/agentic_mcp.py`:
- Line 325: Inside the async context where Agentic variables are logged, the
call to logger.adebug is not awaited (logger.adebug(f"Agentic variables:
{list(agentic_variables.keys())}")), causing inconsistent async behavior with
other awaits; change this call to use await logger.adebug(...) so it matches the
surrounding awaited logger calls (see other occurrences of await logger.adebug
in this file) and ensure the statement remains inside the same async function
that references agentic_variables.
In `@src/backend/tests/locust/langflow_setup_test.py`:
- Line 374: The export command currently prints a masked API key with
print(f"export API_KEY='***{setup_state['api_key'][-4:]}'"), which prevents
copy-paste use; update the print to emit the full API key value from
setup_state['api_key'] so the export command is immediately usable (or, if
redaction is required, change the export print to instruct users to paste the
previously-displayed full key, e.g., print("export API_KEY='<use the API key
displayed above>'")). Ensure you modify the print statement that references
setup_state['api_key'] so the exported command contains the actual key or a
clear instruction to retrieve it.
---
Outside diff comments:
In `@src/backend/tests/locust/langflow_setup_test.py`:
- Around line 407-418: The save_credentials function is currently persisting
full sensitive values (password, access_token, api_key) while
print_setup_results redacts them; update save_credentials to either redact these
fields before writing or emit a clear warning/log that full credentials are
being written so reviewers/users are aware. Concretely, in save_credentials
replace or duplicate sensitive fields (password, access_token, api_key) with
masked versions (e.g., keep last 4 chars) before serializing, and/or add a
processLogger.warning call (or raise a flag) in the same function to document
that unredacted credentials will be saved; reference save_credentials and
print_setup_results so the change is consistent with the redaction behavior.
---
Nitpick comments:
In `@src/backend/base/langflow/api/v1/openai_responses.py`:
- Around line 620-622: Remove the redundant debug log that repeats the same keys
already logged: delete the second/third call to logger.adebug that prints
list(variables.keys()) so only one meaningful message remains (e.g., keep
"Extracted global variables from headers: {list(variables.keys())}"); locate the
three consecutive await logger.adebug calls (the ones referencing
http_request.headers and variables) and remove the duplicate "Variables dict
keys" logger call to avoid duplicated output.
In `@src/backend/tests/locust/langflow_setup_test.py`:
- Around line 360-362: Replace the inline conditional print with a simple
if/else for readability: check setup_state["access_token"] and if present print
the masked token using setup_state['access_token'][-4:], otherwise print "JWT
Token: N/A"; update the code around the existing print statement that
references setup_state and access_token (the block currently using the inline
conditional) to use this clearer if/else structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eca215ac-0029-44c5-b592-8e8c787afe04
📒 Files selected for processing (5)
src/backend/base/langflow/__main__.pysrc/backend/base/langflow/api/utils/mcp/agentic_mcp.pysrc/backend/base/langflow/api/utils/mcp/config_utils.pysrc/backend/base/langflow/api/v1/openai_responses.pysrc/backend/tests/locust/langflow_setup_test.py
| # Create a dict with agentic variable names and default values as empty strings | ||
| agentic_variables = dict.fromkeys(AGENTIC_VARIABLES, DEFAULT_AGENTIC_VARIABLE_VALUE) | ||
| logger.adebug(f"Agentic variables: {agentic_variables}") | ||
| logger.adebug(f"Agentic variables: {list(agentic_variables.keys())}") |
There was a problem hiding this comment.
Inconsistent use of sync logger in async function.
Line 325 uses logger.adebug(...) without await, while the surrounding code (e.g., lines 330, 341, 352, 354) correctly uses await logger.adebug(...). This should be awaited for consistency and proper async behavior.
Proposed fix
- logger.adebug(f"Agentic variables: {list(agentic_variables.keys())}")
+ await logger.adebug(f"Agentic variables: {list(agentic_variables.keys())}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.adebug(f"Agentic variables: {list(agentic_variables.keys())}") | |
| await logger.adebug(f"Agentic variables: {list(agentic_variables.keys())}") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/backend/base/langflow/api/utils/mcp/agentic_mcp.py` at line 325, Inside
the async context where Agentic variables are logged, the call to logger.adebug
is not awaited (logger.adebug(f"Agentic variables:
{list(agentic_variables.keys())}")), causing inconsistent async behavior with
other awaits; change this call to use await logger.adebug(...) so it matches the
surrounding awaited logger calls (see other occurrences of await logger.adebug
in this file) and ensure the statement remains inside the same async function
that references agentic_variables.
| print("# Set environment variables:") | ||
| print(f"export LANGFLOW_HOST='{setup_state['host']}'") | ||
| print(f"export API_KEY='{setup_state['api_key']}'") | ||
| print(f"export API_KEY='***{setup_state['api_key'][-4:]}'") |
There was a problem hiding this comment.
Masked API key in export command defeats copy-paste purpose.
The export command is meant to be copied and pasted by users to set environment variables. Masking the API key here makes the command non-functional. Users would need to manually retrieve the actual API key value.
Consider keeping the full API key in the export command since users explicitly run this setup script to obtain working credentials, or provide the value in a separate secure output section.
Proposed fix - keep full API key in export command
- print(f"export API_KEY='***{setup_state['api_key'][-4:]}'")
+ print(f"export API_KEY='{setup_state['api_key']}'")Alternatively, if redaction is required, inform users where to find the full key:
print("export API_KEY='<use the API key displayed above>'")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/backend/tests/locust/langflow_setup_test.py` at line 374, The export
command currently prints a masked API key with print(f"export
API_KEY='***{setup_state['api_key'][-4:]}'"), which prevents copy-paste use;
update the print to emit the full API key value from setup_state['api_key'] so
the export command is immediately usable (or, if redaction is required, change
the export print to instruct users to paste the previously-displayed full key,
e.g., print("export API_KEY='<use the API key displayed above>'")). Ensure you
modify the print statement that references setup_state['api_key'] so the
exported command contains the actual key or a clear instruction to retrieve it.
|
can you merge this into 1.9.0 instead ? |
Mask or remove API keys, passwords, tokens, auth settings, and configuration values from logger calls and print statements to prevent clear-text exposure of credentials in logs.
- Restore full API key display in Windows fallback banner (masking defeated the purpose of showing the key for the only time) - Revert test helper masking in locust setup (developers need full credentials for load testing) - Add missing await on logger.adebug in agentic_mcp - Remove redundant duplicate log line in openai_responses
e594be2 to
6e673ea
Compare
* fix: redact sensitive information from log output Mask or remove API keys, passwords, tokens, auth settings, and configuration values from logger calls and print statements to prevent clear-text exposure of credentials in logs. * fix: address code review feedback on sensitive info redaction - Restore full API key display in Windows fallback banner (masking defeated the purpose of showing the key for the only time) - Revert test helper masking in locust setup (developers need full credentials for load testing) - Add missing await on logger.adebug in agentic_mcp - Remove redundant duplicate log line in openai_responses * fix: replace unnecessary dict comprehension with dict() call --------- Co-authored-by: Janardan Singh Kavia <janardankavia@ibm.com>
Summary
awaiton async logger call in agentic_mcpNote: Test helper output and the API key banner intentionally show full values, as they are meant for developer use (load testing setup, one-time key display).