From cb4f000e57bb482d71cebf702efc821a17ab047d Mon Sep 17 00:00:00 2001 From: Petr Date: Tue, 23 Jun 2026 20:07:25 +0200 Subject: [PATCH] fix(encrypt): name leaked secret key-paths on plaintext fallback (GHSA-7jrf) When --allow-plaintext-on-encrypt-failure is set and the Encryption API fails, encrypt_secrets_in_config fell back to writing plaintext with only a vague `logger.warning("... plaintext fallback allowed")` -- the user was not told which secrets actually went out in plaintext. The fallback warning now enumerates the exact secret key-PATHS persisted in plaintext (via find_plaintext_secret_keys -- paths only, never the values) and states the remediation (rotate + re-encrypt; config version history keeps the plaintext copy). Central fix in the shared helper, so every caller (config update / row, variables-set, data-app secrets, sync push) gets it. Code-only (no version bump / changelog) per the conflict-immunity rationale; version + changelog at next release. Private advisory GHSA-7jrf-xc86-8wf6. --- src/keboola_agent_cli/services/_encryption.py | 23 ++++++++- tests/test_encryption.py | 49 +++++++++++++++++++ tests/test_sync_encrypt.py | 9 ++-- 3 files changed, 76 insertions(+), 5 deletions(-) diff --git a/src/keboola_agent_cli/services/_encryption.py b/src/keboola_agent_cli/services/_encryption.py index 388ebf06..8a02883b 100644 --- a/src/keboola_agent_cli/services/_encryption.py +++ b/src/keboola_agent_cli/services/_encryption.py @@ -182,10 +182,29 @@ def encrypt_secrets_in_config( logger.info("Encrypted %d secret value(s) for %s", len(encrypted), component_id) except Exception as exc: if allow_plaintext_fallback: + # The user opted into --allow-plaintext-on-encrypt-failure, but the + # actual plaintext write is otherwise silent (GHSA-7jrf-xc86-8wf6). + # Name the exact secret key-paths now persisted in PLAINTEXT (never + # the values) so the leak is visible and actionable rather than a + # vague "fallback allowed" log line. + leaked = find_plaintext_secret_keys(configuration) + # An HTTP client's exception message can echo the request body (the + # plaintext secrets we just sent to encrypt_values), so redact every + # secret value from the logged exception -- the warning that exists + # to flag a plaintext leak must not itself leak a credential. + safe_exc = str(exc) + for secret_value in secrets.values(): + if secret_value: + safe_exc = safe_exc.replace(secret_value, "***") logger.warning( - "Failed to encrypt secrets for %s: %s (plaintext fallback allowed)", + "Encryption FAILED for %s (%s). --allow-plaintext-on-encrypt-failure " + "is set, so %d secret value(s) are being written in PLAINTEXT: %s. " + "Rotate these credentials and re-encrypt once the Encryption API is " + "reachable -- config version history retains the plaintext copy.", component_id, - exc, + safe_exc, + len(leaked), + ", ".join(leaked) or "(unable to enumerate)", ) else: raise KeboolaApiError( diff --git a/tests/test_encryption.py b/tests/test_encryption.py index 2bbf7d0e..046c68b1 100644 --- a/tests/test_encryption.py +++ b/tests/test_encryption.py @@ -177,6 +177,55 @@ def test_fail_closed_on_encryption_failure(self) -> None: # plaintext must not be in the config after the raise path either # (it's in place, but the caller must not push it — that's the contract) + def test_plaintext_fallback_warning_names_leaked_keys( + self, caplog: pytest.LogCaptureFixture + ) -> None: + # GHSA-7jrf: with --allow-plaintext-on-encrypt-failure AND a failing + # Encryption API, the (otherwise silent) plaintext write must be + # surfaced -- naming the exact leaked key-PATHS, never the values. + client = MagicMock() + client.encrypt_values.side_effect = RuntimeError("api down") + config = {"parameters": {"#api_key": "s3cr3t-value"}} + + with caplog.at_level("WARNING"): + encrypt_secrets_in_config( + client=client, + project_id=901, + component_id="keboola.ex-generic", + configuration=config, + allow_plaintext_fallback=True, + ) + + msg = caplog.text + assert "PLAINTEXT" in msg + assert "#parameters.#api_key" in msg # the key PATH is named + assert "s3cr3t-value" not in msg # the secret VALUE is NEVER logged + + def test_plaintext_fallback_redacts_secret_value_echoed_in_exception( + self, caplog: pytest.LogCaptureFixture + ) -> None: + # GHSA-7jrf (Devin follow-up): even if the encryption client's exception + # message echoes the request body, the warning must redact the secret + # VALUE -- str(exc) is the remaining transitive leak vector. + client = MagicMock() + client.encrypt_values.side_effect = RuntimeError( + "400 Bad Request while sending {'#api_key': 's3cr3t-value'}" + ) + config = {"parameters": {"#api_key": "s3cr3t-value"}} + + with caplog.at_level("WARNING"): + encrypt_secrets_in_config( + client=client, + project_id=901, + component_id="keboola.ex-generic", + configuration=config, + allow_plaintext_fallback=True, + ) + + assert "s3cr3t-value" not in caplog.text # value redacted out of exc + assert "***" in caplog.text # redaction marker present + assert "#parameters.#api_key" in caplog.text # key path still named + def test_noop_when_no_secrets(self) -> None: client = MagicMock() config = {"values": [{"name": "regular", "value": "public"}]} diff --git a/tests/test_sync_encrypt.py b/tests/test_sync_encrypt.py index 012afbce..cb83077f 100644 --- a/tests/test_sync_encrypt.py +++ b/tests/test_sync_encrypt.py @@ -104,9 +104,12 @@ def test_encrypt_secrets_warns_on_failure_with_fallback( assert result["parameters"]["#apiToken"] == original_token assert result["parameters"]["nested"]["#password"] == "super-secret" - # Warning was logged - assert any("Failed to encrypt secrets" in record.message for record in caplog.records) - assert any("plaintext fallback allowed" in record.message for record in caplog.records) + # Warning now names the leaked key-paths written in PLAINTEXT + # (GHSA-7jrf) -- never the secret values. + assert "Encryption FAILED" in caplog.text + assert "PLAINTEXT" in caplog.text + assert "#parameters.#apiToken" in caplog.text + assert "super-secret" not in caplog.text class TestEncryptSecretsSuccess: