Skip to content

Conversation

@moonbox3
Copy link
Contributor

@moonbox3 moonbox3 commented Feb 2, 2026

Potential fix for https://github.com/microsoft/agent-framework/security/code-scanning/49

In general, to fix clear‑text logging of sensitive information, remove or mask sensitive values from log messages. Logs should contain enough context (such as an identifier or generic description) to debug issues without exposing secrets (passwords, API keys, tokens, etc.).

In this specific case, the problem is the two logger.debug calls in _try_powerfx_eval that interpolate value (or value[:5]) directly into the log message. The best fix while preserving functionality is:

  • Keep the detailed exception message (exc) and note that some value failed evaluation.
  • Remove value entirely from logs, or replace it with a constant placeholder such as '<redacted>'.
  • Continue to use the log_value flag only to control the presence of any value-specific hint vs. a more generic message, but no longer log actual characters of the value.

Given we must not assume any broader project context or modify other files, the change will be localized to _try_powerfx_eval in python/packages/declarative/agent_framework_declarative/_models.py. We will replace:

if log_value:
    logger.debug(f"PowerFx evaluation failed for value '{value}': {exc}")
else:
    logger.debug(f"PowerFx evaluation failed for value (first five characters shown) '{value[:5]}': {exc}")

with something like:

if log_value:
    logger.debug("PowerFx evaluation failed for a value: %s", exc)
else:
    logger.debug("PowerFx evaluation failed for a value (details redacted): %s", exc)

or equivalent f-strings that do not include value. This avoids logging any part of the possibly sensitive data while still preserving error information via the exception.

No new imports or additional helper methods are required.


Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…ensitive information

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@github-actions github-actions bot changed the title Potential fix for code scanning alert no. 49: Clear-text logging of sensitive information Python: Potential fix for code scanning alert no. 49: Clear-text logging of sensitive information Feb 2, 2026
@moonbox3 moonbox3 changed the title Python: Potential fix for code scanning alert no. 49: Clear-text logging of sensitive information Python: improve logging in declarative code Feb 2, 2026
@moonbox3 moonbox3 self-assigned this Feb 2, 2026
@moonbox3 moonbox3 marked this pull request as ready for review February 2, 2026 06:40
Copilot AI review requested due to automatic review settings February 2, 2026 06:40
@markwallace-microsoft
Copy link
Member

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
TOTAL16250198287% 
report-only-changed-files is enabled. No files were changed during this commit :)

Python Unit Test Overview

Tests Skipped Failures Errors Time
3742 221 💤 0 ❌ 0 🔥 1m 8s ⏱️

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a security vulnerability where sensitive information (such as API keys) could be logged in clear text during PowerFx evaluation failures. The fix redacts the actual values from log messages while preserving exception information for debugging.

Changes:

  • Updated the log_value parameter documentation to reflect its new purpose of controlling additional context rather than logging the actual value
  • Replaced direct value logging in error messages with generic placeholders that don't expose sensitive data
  • Maintained differentiation between regular fields and sensitive fields through distinct log messages

@moonbox3 moonbox3 enabled auto-merge February 3, 2026 01:43
@moonbox3 moonbox3 added this pull request to the merge queue Feb 3, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants