fix(data-app): accept the Azure KBC::ProjectSecureKV:: ciphertext prefix (#607) - #611
Merged
Conversation
…fix (#607) The Encryption API returns project-scoped ciphertext under a different prefix per cloud: KBC::ProjectSecure:: (AWS), KBC::ProjectSecureGKMS:: (GCP) and KBC::ProjectSecureKV:: (Azure Key Vault). The fail-closed whitelist ENCRYPTED_PASSWORD_PREFIXES was missing the Azure variant, so a correctly encrypted value was rejected as "not project-scoped" and the write aborted with ENCRYPTION_FAILED before anything reached Storage. The whitelist is shared by five call sites, so the gap broke more than the reported path: - data-app create --git-pat-env / --git-pat-file (encrypt then validate) - data-app create --git-pat-encrypted (a valid Azure ciphertext refused) - data-app secrets-set (same ENCRYPTION_FAILED abort) - secrets-list --show-fingerprint / secrets-get (empty fingerprint and encryption_prefix for Azure ciphertext) Adding the one prefix repairs all of them. Purely additive: unknown prefixes, look-alikes (KBC::ProjectSecureKVX::), the broader ComponentSecure* / ConfigSecure* scopes and plaintext are still rejected, and plaintext is still never written to Storage. Verified live against connection.north-europe.azure.keboola.com (project 13775): the same `data-app create --git-pat-env` invocation fails with ENCRYPTION_FAILED without the fix and succeeds with it; secrets-set writes a KBC::ProjectSecureKV:: value and secrets-list reports the prefix instead of an empty string. The probe app was deleted afterwards.
This was referenced Aug 19, 2026
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
…ix (#613) Review follow-ups on top of the #607 fix. 1. Do not bump the version here. 0.85.0 stays; the release will be cut in a separate PR. The changelog entries are keyed 0.85.1 so that PR only has to bump. 2. gotchas.md: use the mandated `(since vX.Y.Z)` tag form -- it was the only heading in the file using "fixed in". 3. #613: four surfaces described `kbagent encrypt values` as producing ComponentSecure ciphertext. `client/misc.py::encrypt_values` sends `projectId` AND `componentId`, so the API returns a project-scoped ProjectSecure cipher. The described behaviour was right; the cipher name was not, and that name is what an agent pattern-matches on. variables-workflow.md even printed a literal `KBC::ComponentSecure::` as the value kbagent receives. All four now name the per-cloud prefixes instead of a single literal. 4. `KBC::ProjectSecureKMS::` is dropped from ENCRYPTED_PASSWORD_PREFIXES. It arrived with the tuple in 0.27.0 and was never documented; the platform cipher registry (keboola/keboola-operator internal/encryptor/wrapper/registry.go, mirrored by keboola/object-encryptor) has no such prefix -- the AWS wrapper is named PrefixProjectKMS but emits plain `KBC::ProjectSecure::`. No stack can produce that value, so nothing that worked before stops working; the whitelist just no longer accepts a shape that could never decrypt. Covered by a test.
…#612) ENCRYPTED_PREFIXES listed only the four AWS-form prefixes, so on a GCP or Azure stack is_encrypted_value() returned False for a perfectly good KBC::ProjectSecureGKMS:: / KBC::ProjectSecureKV:: value -- the same defect as #607, in the sync path. The tuple is now generated from the platform's cipher registry (keboola/keboola-operator internal/encryptor/wrapper/registry.go, mirrored by keboola/object-encryptor): 8 scopes x 3 clouds, plus the 3 pre-2019 KBC::*Encrypted== ciphers, 27 in total. A test pins the family so a scope added upstream cannot silently go undetected here. Detection is deliberately inclusive -- this is a detector, not an authorization gate. An unrecognised marker reads as plaintext and stops being redacted from diffs, so under-inclusion is the bug. The narrow fail-closed whitelist that decides what kbagent will WRITE stays separate (ENCRYPTED_PASSWORD_PREFIXES in services/data_app_service.py), and the module docstring now says so. Impact is narrow: is_encrypted_value has a single caller, the ciphertext redaction in sync/diff_engine.py::_normalize, and the #-key check already covered every conventionally-named secret. The plaintext audit behind sync status / doctor sync_secrets never used this helper -- it matches the generic KBC:: prefix via services/_encryption.py::is_already_encrypted and was always cloud-agnostic. One-time effect, noted in the changelog and gotchas: a GCP/Azure working tree holding an encrypted value under a NON-# key now normalises it to the <ENCRYPTED> placeholder, so that config hashes as changed on the first run after the upgrade.
padak
commented
Aug 19, 2026
padak
left a comment
padak
left a comment
Member
Author
There was a problem hiding this comment.
Review of #611 — fix(data-app): accept the Azure KBC::ProjectSecureKV:: ciphertext prefix (#607)
Generated by
kbagent-pr-reviewersubagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed viamake check, not duplicated here.
Summary
This PR closes three related issues in one coherent change: (1) adds the missing Azure KBC::ProjectSecureKV:: prefix to the data-app fail-closed whitelist (ENCRYPTED_PASSWORD_PREFIXES) so Azure-stack data apps with private git repos / secrets can be created at all; (2) widens sync/secrets.py::ENCRYPTED_PREFIXES from 4 hand-listed AWS-form prefixes to a 27-entry family (8 scopes × 3 clouds + 3 legacy ciphers) generated from the platform's own cipher registry, fixing GCP/Azure ciphertext being misread as plaintext during sync diff/sync status; (3) corrects four docs surfaces that mislabelled the cipher as ComponentSecure when it is actually project-scoped ProjectSecure. It also removes KBC::ProjectSecureKMS:: from the data-app whitelist as a never-real prefix. Verdict: APPROVE. I independently fetched and decoded keboola/keboola-operator's internal/encryptor/wrapper/registry.go and it is a byte-for-byte match to both the new sync/secrets.py cipher family (24 cloud-scoped + 3 legacy = 27) and the claim that no wrapper emits KBC::ProjectSecureKMS:: (the AWS wrapper is literally named PrefixProjectKMS but its emitted constant is "KBC::ProjectSecure::"), so the one narrowing change in this PR is safe. make check is green (5595 passed, 0 failed), no layer violations, no new commands (so OPERATION_REGISTRY/CLAUDE.md/server-routers are correctly untouched), and the changelog/gotchas/plugin docs are consistently updated under the deliberately-deferred 0.85.1 key.
Verdict
- Verdict: APPROVE
- Blocking findings: 0
- Non-blocking findings: 1
- Nits: 0
Blocking findings
(none)
Non-blocking findings
[NB-1] plugins/kbagent/agents/keboola-expert.md — no §3 inline gotcha for the Azure ciphertext fix
gotchas.md, encrypt-workflow.md, variables-workflow.md, commands-reference.md and AGENT_CONTEXT (context.py) were all correctly updated, but keboola-expert.md §3 Inline Gotchas has no entry pointing an agent at "if ENCRYPTION_FAILED fires on Azure, check the kbagent version before reaching for --allow-plaintext-on-encrypt-failure" — exactly the guidance gotchas.md now gives. This is defensible: the file is currently 60,656 bytes against a ~62KB hard budget (verified via wc -c), CONTRIBUTING.md says to trim before adding, and this is a narrow Azure-only bug fix rather than a new capability, so per the Plugin synchronization map this surface is correctly NON-BLOCKING. Flagging only so the author can make an explicit call rather than an implicit omission.
(none further)
Nits
(none)
Verification log
git rev-parse --abbrev-ref HEAD→claude/cli-issue-607-554bf9, matches<branch>— nogh pr checkoutneeded, working tree already correct.gh auth status→ authenticated aspadak.- Read
CONTRIBUTING.md§"Checklist: Adding a New CLI Command", §"Plugin synchronization map", §"Releasing a new version";CLAUDE.mdconvention #17 +## All CLI Commands;plugins/kbagent/agents/keboola-expert.md§1/§2/§3 — all as required pre-reads. gh pr view 611 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state→ OPEN,+430/-17, 10 files, conventionalfix(data-app):title matching change type ✓. 3 commits (f013a31fix(sync),5abbef8fix(docs),2cdfce3fix(data-app)), each one logical change, allfix:prefix ✓.gh pr diff 611→ 601-line diff reviewed in full; nocommands/*.py,client/*.py, orpermissions.pytouched — confirms no new CLI surface, soOPERATION_REGISTRY/CLAUDE.mdcommand list / server routers correctly require no update.- Layer-violation greps (typer/click in
services/, httpx incommands/, formatter in clients) → all empty ✓. - Magic-number / raw error-code-string / bare-except /
print()/ new-tuple-return / token-leakage greps on the diff → all empty ✓ (all#password/auth="password"hits are test fixture literals or dict keys, not credential leaks). - Independent verification of the removal claim: fetched
keboola/keboola-operatorinternal/encryptor/wrapper/registry.goviagh api repos/keboola/keboola-operator/contents/...and decoded the base64 content directly. Confirmed: exactly 8 AWS-form + 8 Azure(KV)-form + 8 Google(GKMS)-form = 24 cloud-scoped constants, plus 3 legacy==-terminated constants = 27 total, matchingsync/secrets.py's_CIPHER_SCOPES(8) ×_CIPHER_CLOUD_SUFFIXES(3) +_LEGACY_CIPHER_PREFIXES(3) exactly, scope-name-for-scope-name and suffix-for-suffix. Also confirmedPrefixProjectKMS = "KBC::ProjectSecure::"(Go constant name contains "KMS", literal string does not) — independently corroborates the PR's claim thatKBC::ProjectSecureKMS::(the removed prefix) has never been emitted by any wrapper. grep -rn "ProjectSecureKMS"across the repo → only appears in the new tests (asserting rejection), the new code comment explaining removal, and the changelog entry — no other production reference depended on it; historicalchangelog.py0.27.0/0.29.0 entries never documentedProjectSecureKMSeither, consistent with "arrived undocumented, never real."grep -rn "is_encrypted_value\b"/"ENCRYPTED_PASSWORD_PREFIXES"→ confirmed single production caller ofis_encrypted_valueissync/diff_engine.py::_normalize(plus an internal use insidefind_encrypted_paths, which itself has no production caller — only exercised by tests), matching the PR description's "narrow impact" claim;ENCRYPTED_PASSWORD_PREFIXEShas 6 call sites indata_app_service.py, all consistently updated.wc -c plugins/kbagent/agents/keboola-expert.md→ 60,656 bytes (near the ~62KB budget noted in project history) — supports NB-1's context that omitting a new matrix/gotcha row here was a reasonable trade-off, not an oversight.make check(background, ~2m) → exit 0,5595 passed, 12 skipped, 152 deselectedin 126.43s, no FAILED lines. Sincemake checkruns lint → format → typecheck → skill-check → version-sync → changelog-check → error-codes → sentinel-guards → test sequentially and aborts on first failure, reaching the full test run confirms all earlier static gates passed too.git diff main...HEAD --stat -- pyproject.toml plugins/kbagent/.claude-plugin/plugin.json→ empty, confirming the PR intentionally does NOT bump the version (per task focus, changelog entries are correctly pre-keyed0.85.1for a separate release PR to pick up).- Could not independently re-run the PR's live A/B against
connection.north-europe.azure.keboola.com(no Azure test project available in this session; per repo convention, AI agents never handle API tokens) — relying on the PR author's documented live verification (create + secrets-set, with/without fix) and the new unit tests, which do exercise the same code paths deterministically.
Open questions for the author
(none)
#607) Review NB-1: every other agent surface was updated, but the specialist subagent prompt had no §3 trigger for the one thing that will actually bite a session -- ENCRYPTION_FAILED on Azure reads like a broken token or a broken Encryption API, and the tempting next move is --allow-plaintext-on-encrypt-failure, which writes the PAT to Storage in clear. One-line trigger pointing at gotchas.md, per the file's own rule. Budget: 60,656 -> 61,021 bytes, still under the ~62KB cap.
Member
Author
|
NB-1 addressed in 4deca30 — Making the call explicitly rather than leaving it implicit, as the finding asked. |
…554bf9 # Conflicts: # src/keboola_agent_cli/changelog.py
padak
added a commit
that referenced
this pull request
Aug 19, 2026
…dget `main` grew `keboola-expert.md` from 60656 to 61622 bytes while this branch was open (#611, #616). The branch's own copy was fine at 61178, but CI builds the PR MERGE commit -- 61622 + this branch's 522-byte matrix row = 62144, i.e. 144 over the hard 62000 ceiling. All three test jobs failed on it; the branch in isolation passed, which is why it only showed up on the PR. The budget test's own comment rules out raising the ceiling ("split keboola-expert into per-domain specialists rather than raising the ceiling again"), so the row is trimmed to 377 bytes instead: command, version gate, the one fact that changes a decision (recipients live in a separate service, not the flow config), the fallback, and both ways to get it wrong. The flag list and the longer gloss are dropped -- `--help` and `gotchas.md` carry them. The file now sits at 61999 bytes, one under the ceiling. That is not headroom; the next PR touching this file hits the same wall regardless of what it adds. The structural fix the test comment asks for is out of scope here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #607. Closes #612. Closes #613.
Root cause (#607)
The Encryption API returns project-scoped ciphertext under a different prefix per cloud:
KBC::ProjectSecure::KBC::ProjectSecureGKMS::KBC::ProjectSecureKV::ENCRYPTED_PASSWORD_PREFIXESinservices/data_app_service.pywas missing the Azure variant, so a correctly encrypted value was rejected as "not project-scoped" and the write aborted withENCRYPTION_FAILEDbefore anything reached Storage.Wider blast radius than the issue reports
The whitelist is shared by five call sites, so the gap broke more than the reported path:
data-app create --git-pat-env/--git-pat-file— encrypt-then-validate (the reported failure)data-app create --git-pat-encrypted— a valid Azure ciphertext was refused as inputdata-app secrets-set— sameENCRYPTION_FAILEDabortdata-app secrets-list --show-fingerprint/secrets-get— emptyfingerprintandencryption_prefixfor Azure ciphertextAdding the one prefix repairs all of them.
Also in this PR
KBC::ProjectSecureKMS::removed from the whitelist. It arrived with the tuple in 0.27.0 and was never documented. The platform's cipher registry — keboola/keboola-operatorinternal/encryptor/wrapper/registry.go, mirrored by keboola/object-encryptor — has no such prefix; the AWS wrapper is namedPrefixProjectKMSwhile the prefix it emits is plainKBC::ProjectSecure::. No stack can produce that value, so nothing that worked before stops working; the whitelist just no longer accepts a shape that could never decrypt.#613 — docs named the wrong cipher. Four surfaces described
kbagent encrypt valuesas producing ComponentSecure ciphertext.client/misc.py::encrypt_valuessendsprojectIdandcomponentId, so the API returns a project-scoped ProjectSecure cipher. The described behaviour (works in any config of that component, survives config clone and branch merge) was right; only the cipher name was wrong — and that name is what an agent pattern-matches on.variables-workflow.mdeven printed a literalKBC::ComponentSecure::...as the value kbagent receives. Corrected inencrypt-workflow.md,variables-workflow.md,commands-reference.mdand thekbagent contextAGENT_CONTEXT, all now naming the per-cloud prefixes instead of a single literal.#612 — the sync path had the same gap.
sync/secrets.py::ENCRYPTED_PREFIXESlisted only the four AWS-form prefixes, so on GCP/Azureis_encrypted_value()returnedFalsefor a perfectly good ciphertext. The tuple is now generated from the platform registry — 8 scopes × 3 clouds + the 3 pre-2019KBC::*Encrypted==ciphers, 27 in total — with a test pinning the family so an upstream scope cannot silently go undetected. Detection there is deliberately inclusive (it is a detector, not an authorization gate; an unrecognised marker stops being redacted from diffs), and the module docstring now says so and points at the separate narrow write whitelist.Impact was narrow:
is_encrypted_valuehas a single caller, the redaction insync/diff_engine.py::_normalize, and the#-key check already covered every conventionally-named secret. The plaintext audit behindsync status/doctor sync_secretsnever used this helper — it goes throughservices/_encryption.py::is_already_encrypted, matches the genericKBC::prefix, and was always cloud-agnostic. One-time effect: a GCP/Azure working tree holding an encrypted value under a non-#key now normalises to<ENCRYPTED>, so that config hashes as changed on the first run after the upgrade (noted in changelog + gotchas).No version bump.
pyproject.tomlstays at 0.85.0 — the release is cut in a separate PR. The changelog entries are keyed0.85.1so that PR only has to bump.Safety
Additive for Azure; narrowing only for a cipher shape that does not exist. Unknown prefixes, look-alikes (
KBC::ProjectSecureKVX::), the broaderComponentSecure*/ConfigSecure*/ProjectWideSecure*scopes (not project-bound) and plaintext are all still rejected, and plaintext is still never written to Storage.Verification
Live A/B against
connection.north-europe.azure.keboola.com(project 13775). The samedata-app create --git-pat-envinvocation:ENCRYPTION_FAILED: Encryption API did not return a project-scoped ciphertext for the git PATKBC::ProjectSecureKV::ciphertextsecrets-setthen wrote aKBC::ProjectSecureKV::value andsecrets-list --show-fingerprintreportedencryption_prefix: KBC::ProjectSecureKVinstead of an empty string. The probe app was deleted afterwards.Unit tests — new
TestAzureKeyVaultCiphertextintests/test_data_app_service.py:--git-pat-encryptedaccepts an Azure ciphertextsecrets-setaccepts an Azure ciphertextENCRYPTION_FAILEDand cleans up the shell appKBC::ProjectSecureKMS::is rejected_secret_fingerprint/_derive_encryption_prefixrecognise the Azure prefixtests/test_sync_secrets.pygains per-cloud, legacy-cipher, look-alike-rejection and family-completeness cases.make checkgreen.