Skip to content

fix: leak of keys on logs during exception#2100

Merged
edwinjosechittilappilly merged 1 commit into
release-saas-ga-0.6.2from
add-logs-filters
Jul 15, 2026
Merged

fix: leak of keys on logs during exception#2100
edwinjosechittilappilly merged 1 commit into
release-saas-ga-0.6.2from
add-logs-filters

Conversation

@edwinjosechittilappilly

Copy link
Copy Markdown
Collaborator

This pull request strengthens the application's protection against leaking sensitive information (such as API keys and JWTs) into logs. It does so by improving documentation, updating logging configuration to avoid serializing local variables in exception tracebacks, and adding unit tests to ensure secrets are never exposed in production logs.

Logging and Security Improvements:

  • Updated comments and documentation in src/agent.py, src/services/chat_service.py, and src/utils/langflow_headers.py to warn developers never to log or pass header dictionaries (which may contain secrets) to loggers, and to use sanitization if logging is required. [1] [2] [3] [4] [5]

Logging Configuration Changes:

  • Modified configure_logging in src/utils/logging_config.py to ensure production JSON logs do not serialize exception-frame locals, which could leak secrets. Now uses ExceptionRenderer with show_locals=False for structured tracebacks.

Testing and Validation:

  • Added unit tests in tests/unit/utils/test_logging_config.py to verify that JSON exception logs do not leak local variables (such as API keys) and still include necessary frame information for debugging.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 38243191-3d80-4a0b-91fb-0d844f44c115

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch add-logs-filters

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) tests labels Jul 15, 2026
@edwinjosechittilappilly edwinjosechittilappilly changed the title fix : leak of keys on logs during exception fix: leak of keys on logs during exception Jul 15, 2026
@github-actions github-actions Bot added the bug 🔴 Something isn't working. label Jul 15, 2026
@edwinjosechittilappilly

Copy link
Copy Markdown
Collaborator Author

@coderabbitai check the risk of merging this PR will it effect any functionality?

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
gh pr view 2100 --repo langflow-ai/openrag --json title,body,files,additions,deletions,changedFiles
echo "-----DIFF-----"
gh pr diff 2100 --repo langflow-ai/openrag

Length of output: 9614


🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n "structlog" -g "*.txt" -g "*.toml" -g "*.lock" -g "*.cfg" | head -50
echo "---"
fd -e py logging_config.py
echo "---"
cat src/utils/logging_config.py | sed -n '1,60p'
echo "---rest---"
sed -n '120,180p' src/utils/logging_config.py

Length of output: 574


🏁 Script executed:

#!/bin/bash
set +e
rg -n "structlog" pyproject.toml requirements*.txt 2>/dev/null
echo "---files---"
fd -e py -a logging_config.py
echo "---content---"
sed -n '1,200p' src/utils/logging_config.py
echo "---pyproject---"
find . -maxdepth 2 -iname "pyproject.toml" -exec cat {} \;

Length of output: 12921


@edwinjosechittilappilly I reviewed the full diff for #2100. Risk is low — this PR is almost entirely comments/docstrings plus one narrowly-scoped logging behavior change.

Breakdown of the 5 changed files:

  • src/agent.py (+5/0), src/services/chat_service.py (+12/-3), src/utils/langflow_headers.py (+4/0): purely comment/docstring additions warning against logging header dicts. Zero functional change — no code paths, control flow, or logic touched.
  • tests/unit/utils/test_logging_config.py (new, +59): net-new test file, doesn't touch runtime code.
  • src/utils/logging_config.py (+8/-2): the only actual behavior change. Inside configure_logging(), the JSON-logging branch (use_json == True, i.e. production) swaps:
    structlog.processors.dict_tracebacks
    for
    structlog.processors.ExceptionRenderer(
        structlog.tracebacks.ExceptionDictTransformer(show_locals=False)
    )
    This is scoped only to the production/JSON logging branch — the human-readable dev logging path is untouched.

Why this is safe:

  • structlog.processors.dict_tracebacks is itself implemented internally as ExceptionRenderer(ExceptionDictTransformer()) with show_locals=True by default, so the JSON schema of the "exception" field (stack list, frames, exc_type, etc.) stays identical — only the locals sub-key inside each frame becomes empty/absent.
  • structlog>=25.4.0 is already a pinned dependency, and both ExceptionRenderer and ExceptionDictTransformer are stable, long-standing public structlog APIs — no dependency/version compatibility concern.
  • No changes to request handling, header construction, authentication/JWT logic, or any other business logic.

Only trade-off to be aware of: if any log dashboards (ELK/Datadog/Splunk) or alerting rules specifically parse exception[].frames[].locals for debugging context, that data will now be empty going forward — this is the intended security fix (preventing secret leakage), not a bug.

Net: merging should not affect any existing functionality other than removing local-variable data from exception tracebacks in production JSON logs, which is the intended fix.

(๑˃̵ᴗ˂̵)و

@edwinjosechittilappilly

Copy link
Copy Markdown
Collaborator Author

@coderabbitai any change sin production env required ?

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
gh pr diff 2100 --repo langflow-ai/openrag -- src/utils/logging_config.py

Length of output: 448


🏁 Script executed:

#!/bin/bash
set -euo pipefail
gh pr diff 2100 --repo langflow-ai/openrag | sed -n '/logging_config.py/,/diff --git/p'

Length of output: 3647


@edwinjosechittilappilly No production environment changes are required for this PR.

  • The behavior change is fully internal to configure_logging() in src/utils/logging_config.py, gated by the same existing use_json flag your app already uses to switch between dev/production logging — no new environment variables, config keys, or feature flags are introduced.
  • No new dependency is added; structlog.processors.ExceptionRenderer and structlog.tracebacks.ExceptionDictTransformer are existing public APIs from the already-pinned structlog package.
  • The function's call signature is unchanged, so nothing needs to be updated where configure_logging(...) is invoked at startup.
  • Everything else in the diff is comments/docstrings or the new test file — no runtime impact.

The only thing to be aware of operationally (not a required change, just informational): once deployed, production JSON exception logs will no longer include the exception[].frames[].locals field. If you have any log dashboards/alerts in ELK/Datadog/Splunk that specifically query that field for debugging context, that data will simply be absent going forward — this is the intended security fix, so no action is needed unless you want to update such dashboards to stop expecting it.

@github-actions github-actions Bot added the lgtm label Jul 15, 2026

@mpawlow mpawlow left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review 1

  • ✅ Approved / LGTM 🚀

@edwinjosechittilappilly
edwinjosechittilappilly merged commit c0a8435 into release-saas-ga-0.6.2 Jul 15, 2026
37 of 48 checks passed
@github-actions
github-actions Bot deleted the add-logs-filters branch July 15, 2026 20:32
edwinjosechittilappilly added a commit that referenced this pull request Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) bug 🔴 Something isn't working. lgtm tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants