Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion application/single_app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
EXECUTOR_TYPE = 'thread'
EXECUTOR_MAX_WORKERS = 30
SESSION_TYPE = 'filesystem'
VERSION = "0.250.217"
VERSION = "0.250.218"
IS_DEVELOPMENT = is_development_env_enabled()

SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax')
Expand Down
23 changes: 23 additions & 0 deletions application/single_app/functions_appinsights.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,40 @@
"accesstoken",
"accountkey",
"apikey",
"authkey",
"authorization",
"clientsecret",
"connectionstring",
"cookie",
"credential",
"encryptionkey",
"keypair",
"masterkey",
"password",
"primarykey",
"privatekey",
"sas",
"secondarykey",
"secret",
"sessionkey",
"sharedaccesssignature",
"signingkey",
"storagekey",

Check warning on line 41 in application/single_app/functions_appinsights.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains secret or sensitive data source marker. Recommendation%3A Pair this source with any nearby network, logging, serialization, or process execution sink before approving.
"subscriptionkey",
"token",
)
# Names that carry a credential only when they are the whole key. Matching these as

Check warning on line 45 in application/single_app/functions_appinsights.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains secret or sensitive data source marker. Recommendation%3A Pair this source with any nearby network, logging, serialization, or process execution sink before approving.
# substrings would redact benign configuration such as key_encoding or partition_key_path,

Check warning on line 46 in application/single_app/functions_appinsights.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains security control, sanitization, or audit marker. Recommendation%3A Confirm the change does not weaken auth, CSRF, CSP, XSS defenses, settings sanitization, redaction, audit logging, or tests.
# so they are compared against the fully normalized key instead.
SENSITIVE_LOG_KEY_EXACT = (
"key",
"keys",
"pass",

Check warning on line 51 in application/single_app/functions_appinsights.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains security control, sanitization, or audit marker. Recommendation%3A Confirm the change does not weaken auth, CSRF, CSP, XSS defenses, settings sanitization, redaction, audit logging, or tests.
"passphrase",

Check warning on line 52 in application/single_app/functions_appinsights.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains security control, sanitization, or audit marker. Recommendation%3A Confirm the change does not weaken auth, CSRF, CSP, XSS defenses, settings sanitization, redaction, audit logging, or tests.
"pwd",

Check warning on line 53 in application/single_app/functions_appinsights.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains secret or sensitive data source marker. Recommendation%3A Pair this source with any nearby network, logging, serialization, or process execution sink before approving.
"sig",
"signature",
)
EXTERNAL_EVENT_SENSITIVE_KEY_FRAGMENTS = (
"email",
"userid",
Expand Down Expand Up @@ -120,6 +141,8 @@
normalized_key = _normalize_log_key(key)
if not normalized_key:
return False
if normalized_key in SENSITIVE_LOG_KEY_EXACT:
return True
return any(fragment in normalized_key for fragment in SENSITIVE_LOG_KEY_FRAGMENTS)


Expand Down
6 changes: 3 additions & 3 deletions deployers/bicep/postconfig.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# postconfig.py
from azure.cosmos import CosmosClient
import azure.cosmos as azure_cosmos

Check warning on line 2 in deployers/bicep/postconfig.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains secret or sensitive data source marker. Recommendation%3A Pair this source with any nearby network, logging, serialization, or process execution sink before approving.
from azure.cosmos.exceptions import CosmosResourceNotFoundError
from azure.identity import AzureCliCredential
import json
Expand Down Expand Up @@ -291,10 +291,10 @@
cosmosKey = os.getenv("var_cosmosDb_key")

if cosmosKey:
client = CosmosClient(cosmosEndpoint, cosmosKey)
client = azure_cosmos.CosmosClient(cosmosEndpoint, cosmosKey)

Check warning on line 294 in deployers/bicep/postconfig.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains secret or sensitive data source marker. Recommendation%3A Pair this source with any nearby network, logging, serialization, or process execution sink before approving.
else:
credential.get_token("https://cosmos.azure.com/.default")
client = CosmosClient(cosmosEndpoint, credential=credential)
client = azure_cosmos.CosmosClient(cosmosEndpoint, credential=credential)

Check warning on line 297 in deployers/bicep/postconfig.py

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains secret or sensitive data source marker. Recommendation%3A Pair this source with any nearby network, logging, serialization, or process execution sink before approving.

database_name = "SimpleChat"
container_name = "settings"
Expand Down
2 changes: 1 addition & 1 deletion deployers/version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.0.24
1.0.25
116 changes: 116 additions & 0 deletions docs/explanation/fixes/LOG_CREDENTIAL_KEY_REDACTION_FIX.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
# Log Credential Key Redaction Fix

Check warning on line 1 in docs/explanation/fixes/LOG_CREDENTIAL_KEY_REDACTION_FIX.md

View workflow job for this annotation

GitHub Actions / malicious-pr-security-review

Important - Changed line contains secret or sensitive data source marker. Recommendation%3A Pair this source with any nearby network, logging, serialization, or process execution sink before approving.

Fixed/Implemented in version: **0.250.218**

## Issue Description

CodeQL reported five high-severity `py/clear-text-logging-sensitive-data` alerts against the
shared logging sinks in `application/single_app/functions_appinsights.py`, plus two
`py/import-of-mutable-attribute` warnings in helper scripts.

The logging alerts were not false positives. `log_event` sanitizes its inputs before they reach
any sink, but the redaction decision for structured properties was made by
`_is_sensitive_log_key`, which matched a fixed list of substrings. Several credential field names
used by this codebase did not contain any of those substrings and were therefore logged in
clear text.

The most significant gap was **`auth_key`**, which is the field name the plugin/action
connection-test routes use for the caller-supplied secret, and the plugin manifest's
**`auth.key`**, which `plugin.schema.json` describes as holding "the secret value for the plugin
... such as a SQL connection string, a password for a service principal."

Reproduction before the fix:

```text
log_event("credential redaction probe", extra={"auth_key": "SuperSecretCredentialValue123"})
-> [LOG] credential redaction probe -- {'auth_key': 'SuperSecretCredentialValue123'}
```

## Root Cause Analysis

`_normalize_log_key` strips non-alphanumeric characters, so `auth_key`, `authKey`, and
`auth-key` all normalize to `authkey`. The `SENSITIVE_LOG_KEY_FRAGMENTS` tuple contained
`accountkey`, `apikey`, `privatekey`, and `subscriptionkey`, but not `authkey`, and no fragment
is a substring of `authkey`. The same was true for a property named exactly `key`, and for
`pwd`, `key_pair`, `master_key`, `primary_key`, `secondary_key`, `encryption_key`,
`signing_key`, `session_key`, and `storage_key`.

A value under one of these keys was only redacted by luck, when the value itself happened to
match `SECRET_ASSIGNMENT_RE` (for example a connection string containing `Password=`). A bare
API key or token under `auth_key` was emitted verbatim.

Eighteen credential key names were affected in total.

Widening the match to "any key containing `key`" was not acceptable, because it would redact
benign configuration such as `key_encoding`, `key_prefix_hints`, and `partition_key_path`,
removing diagnostic value from logs.

## Technical Details

Files modified:

* `application/single_app/functions_appinsights.py`
* `scripts/resolve_multiendpoint_gpt.py`
* `deployers/bicep/postconfig.py`
* `deployers/version.txt`
* `functional_tests/test_privacy_logging_telemetry_audit.py`
* `functional_tests/test_log_credential_key_redaction.py` (new)

Code changes summary:

* Added the missing credential fragments to `SENSITIVE_LOG_KEY_FRAGMENTS`: `authkey`,
`encryptionkey`, `keypair`, `masterkey`, `primarykey`, `secondarykey`, `sessionkey`,
`signingkey`, and `storagekey`.
* Added a new `SENSITIVE_LOG_KEY_EXACT` tuple for names that carry a credential only when they
are the entire key: `key`, `keys`, `pass`, `passphrase`, `pwd`, `sig`, and `signature`.
`_is_sensitive_log_key` now checks the fully normalized key against this tuple before falling
back to substring matching. Matching these as substrings would have redacted `key_encoding`
and `partition_key_path`, so the exact-match list keeps the fix surgical.
* Replaced the direct `from azure.cosmos import CosmosClient` bindings in the two remaining
helper scripts with `import azure.cosmos as azure_cosmos` and module-qualified
`azure_cosmos.CosmosClient(...)` calls, matching the pattern established in
`COSMOSCLIENT_IMPORT_BINDING_CODEQL_FIX.md` (v0.250.047). No direct `CosmosClient` imports
remain in the repository.
* Restored `functional_tests/test_privacy_logging_telemetry_audit.py`, which had been failing
since v0.242.072 because it asserted an exact `config.py` version. It now uses
`assert_app_version_at_least`, per the repository's version-assertion guidance, so the privacy
audit runs again.

## Validation

Test results:

* `functional_tests/test_log_credential_key_redaction.py` (new): 6/6 passed. Verified to fail
before the fix, reporting all 18 unredacted credential key names and a reproduced `auth_key`
leak, which confirms it is a real regression guard rather than a tautology.
* `functional_tests/test_privacy_logging_telemetry_audit.py`: 5/5 passed, previously erroring
out before running any assertion.
* `functional_tests/test_log_event_call_contract.py`: passed.
* Route policy and plugin suites: passed.

Before and after:

| Property | Before | After |
|---|---|---|
| `{"auth_key": "<secret>"}` | logged in clear text | `***REDACTED***` |
| `{"auth": {"key": "<secret>"}}` | logged unless the value matched a `secret=` pattern | `***REDACTED***` |
| `{"pwd": "<secret>"}` | logged in clear text | `***REDACTED***` |
| `{"key_encoding": "utf8"}` | visible | visible (unchanged) |
| `{"partition_key_path": "/id"}` | visible | visible (unchanged) |

Impact:

* Credential values supplied to action connection tests can no longer reach stdout or
Application Insights in clear text.
* Diagnostic value is preserved: benign configuration keys containing "key" remain readable, and
sensitive keys still report presence through the `<key>_present` property in the structured
log record.
* Runtime behavior is otherwise unchanged.

## Note on the remaining CodeQL alerts

The five `py/clear-text-logging-sensitive-data` alerts point at the logging sinks themselves.
CodeQL does not model `sanitize_log_message` and `sanitize_log_properties` as sanitizers, so it
may continue to report those sinks even though the data reaching them is redacted. This change
addresses the underlying gap the alerts exposed; if the alerts persist, they can be triaged in
the repository's code scanning view with this fix as the justification.
1 change: 1 addition & 0 deletions docs/explanation/fixes/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ category: Version History
- [Public Workspace Prompt Migration Fix](PUBLIC_WORKSPACE_PROMPT_MIGRATION_FIX.md)
- [Azure OpenAI Model Discovery Identity Fix](v0.250.001/AZURE_OPENAI_MODEL_DISCOVERY_IDENTITY_FIX.md)
- [CosmosClient Import Binding CodeQL Fix](COSMOSCLIENT_IMPORT_BINDING_CODEQL_FIX.md)
- [Log Credential Key Redaction Fix](LOG_CREDENTIAL_KEY_REDACTION_FIX.md)
- [Conversation Cache Invalidation Authorization Fix](CONVERSATION_CACHE_INVALIDATION_AUTHORIZATION_FIX.md)
- [Chat Completion Background Unread Guard Fix](CHAT_COMPLETION_BACKGROUND_UNREAD_GUARD_FIX.md)
- [Settings Container RU Write Suppression Fix](SETTINGS_CONTAINER_RU_WRITE_SUPPRESSION_FIX.md)
Expand Down
18 changes: 18 additions & 0 deletions docs/explanation/release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,24 @@

For feature-focused and fix-focused drill-downs by version, see [Features by Version](/explanation/features/) and [Fixes by Version](/explanation/fixes/).

### **(v0.250.218)**

#### Bug Fixes

* **Credential Field Names Logged in Clear Text**
* Fixed a gap where credential values could be written to application logs and Application Insights in clear text. The log redactor matched only a fixed list of key-name substrings, so field names this codebase actually uses for secrets were missed. The most significant were `auth_key`, used by the action connection-test routes for the caller-supplied secret, and the plugin manifest's `auth.key`, which holds connection strings and service principal passwords.
* Eighteen credential key names were affected in total, including `pwd`, `key_pair`, `master_key`, `primary_key`, `secondary_key`, `encryption_key`, `signing_key`, `session_key`, and `storage_key`.
* Benign configuration keys that merely contain the word "key", such as `key_encoding`, `key_prefix_hints`, and `partition_key_path`, deliberately stay visible so logs keep their diagnostic value.
* (Ref: `functions_appinsights.py`, `test_log_credential_key_redaction.py`, `LOG_CREDENTIAL_KEY_REDACTION_FIX.md`)

* **CosmosClient Import Bindings in Helper Scripts**
* Completed the v0.250.047 import-binding cleanup by updating the two remaining scripts that bound `CosmosClient` directly, so patching `azure.cosmos.CosmosClient` is observed consistently. No direct `CosmosClient` imports remain in the repository.
* (Ref: `scripts/resolve_multiendpoint_gpt.py`, `deployers/bicep/postconfig.py`)

* **Privacy Logging Audit Test Restored**
* The privacy logging and telemetry audit had been failing since v0.242.072 because it asserted an exact `config.py` version and never reached its assertions. It now asserts a version floor, per the repository's version-assertion guidance, so the audit runs again.
* (Ref: `test_privacy_logging_telemetry_audit.py`)

### **(v0.250.217)**

#### New Features
Expand Down
Loading
Loading