fix(encrypt): surface leaked secret key-paths on plaintext fallback (GHSA-7jrf, code-only)

[padak]

Summary

Addresses M7 from the 2026-06-12 security audit (private advisory GHSA-7jrf-xc86-8wf6) — the silent plaintext secret write on encryption failure.

When a user passes --allow-plaintext-on-encrypt-failure and the Encryption API then fails, encrypt_secrets_in_config falls back to writing the secrets in plaintext. The only signal was a vague logger.warning("Failed to encrypt secrets for X (plaintext fallback allowed)") — it never said which secrets actually went out in plaintext, so an operator could miss that a real credential just landed unencrypted in config (and in version history).

Fix

The fallback warning now enumerates the exact secret key-paths that were persisted in plaintext, via the existing find_plaintext_secret_keys helper (returns key paths like #parameters.#api_key, never the values), and states the remediation (rotate the credentials + re-encrypt once the Encryption API is reachable; config version history retains the plaintext copy).

This is a central fix in the shared _encryption.encrypt_secrets_in_config helper, so it covers every caller automatically — config update / row-create / row-update, config new --push, variables-set, data-app secrets-set, and the sync push paths — without threading a field through 8+ call sites. The test_sync_encrypt test (which goes through SyncService._encrypt_secrets_in_config) confirms the sync path inherits it.

Scope note (honest)

The advisory also mentions the write is "absent from structured output". This PR makes the warning loud and actionable (names the leaked keys) but keeps it a logger.warning — it does not yet add a plaintext_written: [...] field to each command's --json envelope. Doing that cleanly would thread a structured field through all ~8 service callers + their command layers (a much larger change). I chose the central, caller-agnostic warning as the proportionate fix; happy to follow up with the per-caller --json field if you want the structured-output surface too (it's the right call for the AI-agent/scripted audience).

Tests

test_encryption.py: new test asserts the fallback warning names the key-PATH (#parameters.#api_key) and never the secret value. test_sync_encrypt.py: updated to the new wording + the never-log-the-value invariant. Full suite green: 4178 passed, 135 skipped; lint/format/ty clean.

⚠️ Code-only PR

No version bump / changelog (added at next release) — same conflict-immunity rationale as #422 / #460 / #461 / #462.

Audit progress

This is the last MEDIUM. After M7 + M10 (#462) merge, only the owner-accepted residuals remain (M1 residual: KBC_TOKEN/cli_command stripping; M5: capability-scoped child token), both deferred by design.

---

[devin-ai-integration]

Devin Review found 1 potential issue.

[padak]

Review of #463 — fix(encrypt): surface leaked secret key-paths on plaintext fallback (GHSA-7jrf, code-only)

Generated by kbagent-pr-reviewer subagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed via make check, not duplicated here.

Summary

This PR enhances the plaintext-fallback warning in encrypt_secrets_in_config to
enumerate the exact secret key-paths written in plaintext, using the existing
find_plaintext_secret_keys helper which is confirmed to return only key paths
(never values). The core invariant — never log a secret value — is correctly
enforced both in the implementation and in the new test assertions. The fix is
genuinely central: all service callers that go through encrypt_secrets_in_config
inherit it automatically. Two pre-existing code paths that do NOT route through
that function are unaffected (see NON-BLOCKING findings). make check passes
cleanly (4178 passed, 8 skipped). No new CLI commands introduced, so the Plugin
synchronization map does not apply.

Verdict: APPROVE

Verdict

• Verdict: APPROVE
• Blocking findings: 0
• Non-blocking findings: 2
• Nits: 1

Blocking findings

(none)

Non-blocking findings

[NB-1] src/keboola_agent_cli/services/config_service.py:624-630 — "no project_id" early-return logs count but not key paths

ConfigService._encrypt_secrets_before_write has a separate plaintext fallback
for the case where project_id cannot be resolved (lines 624-630). This path
returns early before calling encrypt_secrets_in_config, so the central fix
does not reach it. The existing warning says "writing %d secret(s) as plaintext" —
it logs the count but not the specific key paths, leaving operators in the same
information gap the PR fixes for the encryption-failure path.

The PR description says the fix covers "every caller automatically", which is
true for the encryption-API-failure branch but not for this project-id-resolution
branch. This is a pre-existing gap, not a regression introduced here. A follow-up
could apply sorted(secrets) (already computed on line 615) to enumerate the same
key paths in the log message there.

[NB-2] src/keboola_agent_cli/services/data_app_service.py:1329-1334 — data-app secrets path does not go through encrypt_secrets_in_config

The PR description and commit message claim the fix covers "data-app secrets".
DataAppService.set_secrets uses EncryptService.encrypt() directly (not
encrypt_secrets_in_config), so the central fix does not reach it. The
existing fallback warning at line 1330 already names sorted(problems) (the
keys that failed ciphertext validation), so the severity is lower — but the
code path diverges from what the description implies and may cause confusion
when the next person audits coverage. No code change needed; the description
could note "data-app secrets-set uses a parallel encryption path not covered
by this central fix".

Nits

• [NIT-1] src/keboola_agent_cli/services/_encryption.py:188-199 — the exc is formatted via %s into the warning. In the normal KeboolaApiError case the exception message contains the Encryption API response body. The current Keboola Encryption API does not echo request bodies back in error responses, so no secret value leaks through exc today. A defensive future-proof measure would be to mention in a comment that exc here is an HTTP error or network exception (response-side), not a value-bearing request exception. Low priority — the test verifies the invariant for the key case and the architecture makes request-body echo-back structurally unlikely.

Verification log

• gh pr view 463 --json title,body,files → 3 files, +42/-4, fix(encrypt): prefix, state OPEN ✓
• git show --stat HEAD in worktree → commit 5664925 matches PR description exactly (3 files, _encryption.py +13/-1, test_encryption.py +24/-0, test_sync_encrypt.py +6/-3) ✓
• grep -E '^\+(from typer|import typer|formatter\.|console\.print)' diff | grep services/ → empty ✓ (no layer violation)
• grep -E '^\+.*error_code\s*=\s*"[A-Z_]+"' diff → empty ✓ (no raw error code strings)
• grep -E '^\+\s*except\s*:' diff → empty ✓ (no bare except)
• grep -E '^\+\s*print\(' diff | grep src/ → empty ✓
• Reviewed find_plaintext_secret_keys implementation: delegates to collect_secrets, returns sorted(secrets) where secrets is dict[str, str] keyed by paths — sorted() on a dict returns sorted keys, never values ✓ (never-log-value invariant confirmed at implementation level)
• Reviewed logger.warning args: %s args are component_id (str), exc (exception from HTTP/network layer), len(leaked) (int), ", ".join(leaked) (key paths from find_plaintext_secret_keys) — no argument contains secret values ✓
• Reviewed _raise_api_error in http_base.py:235-315: constructs KeboolaApiError from the HTTP response body, never echoes the HTTP request body; exc.message therefore cannot contain plaintext secrets from the encrypt request ✓
• Test test_plaintext_fallback_warning_names_leaked_keys asserts "#parameters.#api_key" in msg and "s3cr3t-value" not in msg — both assertions directly test the never-log-value contract ✓
• Test test_encrypt_secrets_warns_on_failure_with_fallback (sync path via SyncService._encrypt_secrets_in_config) asserts "#parameters.#apiToken" in caplog.text and "super-secret" not in caplog.text — sync path inheritance confirmed ✓
• make check → All checks passed! (ruff, ruff format, ty), 4178 passed, 8 skipped ✓
• Plugin synchronization map: no new CLI commands introduced — permissions.py, context.py, CLAUDE.md, commands-reference.md, gotchas.md, keboola-expert.md do not require updates ✓
• No version bump / changelog deferred per PR description (same pattern as #422/#460/#461/#462) — explicitly documented, not flagged as blocking ✓

Open questions for the author

(none)