[stas-schaller]

Summary

Release branch for v1.1.0 — raises the Python floor to 3.9, aligns the Core dependency with v17.2.1, fixes two HIGH severity CVEs, closes four security bugs, and migrates the publish workflow to PyPI Trusted Publishing (OIDC).

Changes

Bug Fixes

• CVE-2026-0994 (KSM-764): protobuf JSON recursion DoS — add protobuf>=6.33.5 direct dependency (CVSS 8.2; protobuf 5.x discontinued, fix requires Python 3.9+)
• CVE-2026-26007 (KSM-833): cryptography subgroup attack on SECT curves — add cryptography>=46.0.5 direct dependency (CVSS 8.2)
• Silent plaintext config on permission denied (KSM-938): when cloudkms.cryptoKeys.get was denied, get_key_details() swallowed the 403 silently, key_purpose_details was never set, and __save_config() swallowed the resulting AttributeError — credentials sat on disk as plaintext JSON with no error surfaced. GCPKeyValueStorage now raises on init when key introspection fails. Merges KSM-938: raise on get_key_details failure to prevent plaintext config #1009.
• Non-standard AES-GCM nonce + MD5 change detection (KSM-943): AES-GCM used PyCryptodome's 128-bit (16-byte) default nonce instead of the NIST SP 800-38D recommended 96-bit (12-byte) nonce; existing encrypted blobs remain readable (nonce length is stored in the blob). Also replaced MD5 with SHA-256 for in-process config change detection. Merges fix(storage): use 12-byte AES-GCM nonce and SHA-256 for change detection (KSM-943) #1010.
• read_storage() live dict reference + decrypt_config() plaintext-on-disk default (KSM-944): read_storage() returned self.config directly, allowing callers to silently corrupt internal state without triggering encryption or a disk write. decrypt_config() defaulted to autosave=True, permanently overwriting the encrypted config file with plaintext credentials when called without arguments. read_storage() now returns a copy; decrypt_config() defaults to autosave=False. Merges fix(storage): fix read_storage() live ref and decrypt_config() plaintext-on-disk default (KSM-944) #1011.

Maintenance

• Raise minimum Python version from 3.6 to 3.9
• Update keeper-secrets-manager-core dependency floor from >=16.6.6 to >=17.2.1
• Add Python 3.13 classifier; remove 3.6, 3.7, 3.8 classifiers
• Remove importlib_metadata from requirements.txt (stdlib since Python 3.8)
• Add Change Log section to README; document cloudkms.cryptoKeys.get as a required IAM permission
• Migrate publish workflow to PyPI Trusted Publishing (OIDC) — drops long-lived API token, adds publish toggle for build+SBOM-only runs and repository-url override for TestPyPI dry-runs

Breaking Changes

• Python 3.6, 3.7, and 3.8 are no longer supported. Users on those versions must remain on v1.0.1.
• decrypt_config() no longer writes plaintext to disk by default — callers relying on the old default must now pass autosave=True explicitly.

Related Issues

• KSM-764, KSM-833, KSM-938, KSM-943, KSM-944
• Merges KSM-938: raise on get_key_details failure to prevent plaintext config #1009, fix(storage): use 12-byte AES-GCM nonce and SHA-256 for change detection (KSM-943) #1010, fix(storage): fix read_storage() live ref and decrypt_config() plaintext-on-disk default (KSM-944) #1011

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	pypi/​certifi@​2026.4.22
	pypi/​grpcio@​1.70.0 ⏵ 1.80.0	+1
	pypi/​grpcio-status@​1.70.0 ⏵ 1.80.0
	pypi/​urllib3@​2.6.3
	pypi/​google-auth@​2.38.0 ⏵ 2.52.0	-5
	pypi/​google-auth@​2.38.0 ⏵ 2.50.0	-5
	pypi/​pycparser@​2.23
	pypi/​cffi@​2.0.0
	pypi/​charset-normalizer@​3.4.7
	pypi/​requests@​2.32.5
	pypi/​requests@​2.34.1
	pypi/​google-api-core@​2.24.1 ⏵ 2.30.3	+1
	pypi/​typing-extensions@​4.15.0
	pypi/​pycparser@​3.0
	pypi/​cryptography@​47.0.0
	pypi/​protobuf@​5.29.3 ⏵ 6.33.6	+1	+22
	pypi/​protobuf@​5.29.3 ⏵ 7.35.0	+1	+22
	pypi/​proto-plus@​1.26.0 ⏵ 1.28.0
	pypi/​proto-plus@​1.26.0 ⏵ 1.27.2
	pypi/​googleapis-common-protos@​1.66.0 ⏵ 1.75.0	+1
	pypi/​idna@​3.15
	pypi/​grpc-google-iam-v1@​0.14.0 ⏵ 0.14.4
	pypi/​pyasn1@​0.6.1 ⏵ 0.6.3		+22
	pypi/​pyasn1-modules@​0.4.1 ⏵ 0.4.2

View full report

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🔴 sdk/python/storage/keeper_secrets_manager_storage_gcp_kms/keeper_secrets_manager_storage_gcp_kms/storage_gcp_kms.py:300-304 — The new KSM-946 threading.RLock wraps read_storage/save_storage/set/delete/delete_all (lines 300-337), but change_key (lines 277-298) — the heaviest mutator on the class — was left outside the lock. It assigns self.gcp_key_config (line 286) and via get_key_details() mutates self.key_purpose_details, self.encryption_algorithm, self.is_asymmetric, then calls __save_config, all touching the same shared state the lock protects. Fix: wrap the change_key body in with self._lock: — RLock is re-entrant so the inner __save_config call is fine.

  Extended reasoning...

  What the bug is

  The KSM-946 fix adds self._lock = threading.RLock() in __init__ (line 52) and wraps read_storage, save_storage, set, delete, and delete_all in with self._lock: (lines 300-337). But change_key at lines 277-298 — sitting in the same class, on the same level, as those wrapped mutators — is the heaviest mutator and is left unprotected.

  change_key does all of the following without acquiring the lock:

  • Reads self.config (line 283), conditionally calls self.load_config() (line 285).
  • Assigns self.gcp_key_config = new_gcp_key_config (line 286).
  • Calls self.get_key_details() (line 287) which mutates self.key_purpose_details, self.encryption_algorithm, and self.is_asymmetric (storage_gcp_kms.py:70-79).
  • Calls self.__save_config({}, force=True) (line 288), which reads all of the above to drive encrypt_buffer routing.

  Every one of these attributes is read by __save_config / encrypt_buffer on the locked code path that set / delete take.

  The race (concrete, observable)

  Starting state: storage initialized with a symmetric ENCRYPT_DECRYPT key. is_asymmetric == False.

  1. Thread A calls change_key(new_key) with a new asymmetric key. It executes line 286 — self.gcp_key_config = new_gcp_key_config — and is preempted before get_key_details() runs. State: gcp_key_config = NEW (asymmetric), is_asymmetric = False (OLD), encryption_algorithm = OLD.
  2. Thread B calls set(KEY_CLIENT_ID, "x"). It acquires self._lock at line 315 (Thread A is not holding it because change_key never tried to acquire it — RLock is a mutex, but mutexes only exclude threads that try to acquire). B enters save_storage → __save_config.
  3. Inside __save_config, encrypt_buffer is invoked with key_properties=self.gcp_key_config (NEW asymmetric key resource) but is_asymmetric=self.is_asymmetric (still False) and encryption_algorithm=self.encryption_algorithm (still OLD).
  4. encrypt_buffer takes the symmetric branch (because is_asymmetric == False) and calls encrypt_data_and_validate_crc / client.encrypt against an asymmetric key resource. GCP rejects the request — and post-PR encrypt_buffer now raises (utils.py:75), so __save_config raises, save_storage raises, set propagates an inscrutable error.

  The mirror-image race — A finishing get_key_details (so is_asymmetric=True) while still in __save_config, with B reading a mix of NEW is_asymmetric + OLD gcp_key_config (if B took a snapshot earlier) — produces the same class of torn-state failure. CPythons GIL serializes single attribute writes but not multi-statement sequences, so torn reads across multiple attributes are real.

  Why the existing code does not prevent it

  This is exactly the race class the PR identified and added the lock for. The PR added the lock to five mutators but missed the sixth. There is no acquire on change_keys path, so the mutex held by B does not exclude A.

  README CHANGELOG advertises the broader contract

  The 1.1.0 CHANGELOG entry this PR adds says: "Fixed thread-safety: added threading.RLock to GCPKeyValueStorage — concurrent set() / delete() calls no longer race on the config dict or the encrypt-and-write sequence (KSM-946)". A reader of that contract would reasonably expect all public mutators on the class to honor the lock. change_key is the one mutator that breaks it.

  Impact

  • Concurrent set/delete/change_key callers can hit a torn-state encrypt_buffer invocation routed through the wrong code path — error or wrong-ciphertext on disk.
  • The class advertises a thread-safety contract (CHANGELOG) that one of its mutators silently violates.

  How to fix

  One line of indentation. Wrap change_keys body in with self._lock: — same pattern the PR established for the five mutators immediately below it. RLock is re-entrant, so the inner __save_config (which itself does no explicit acquire on this path) and the inner load_config call are unaffected:

  def change_key(self, new_gcp_key_config: GCPKeyConfig) -> bool:
      with self._lock:
          old_key_configuration = self.gcp_key_config
          old_crypto_client = self.crypto_client
          try:
              ...
          except Exception as error:
              ...

  Not a duplicate

  The existing review comment about change_key (comment 3228324893 and its repeat at 3229647853) is about the rollback gap — that the except block only restores 2 of the 5 attributes mutated by get_key_details. This bug is about the lock gap — that the function takes no lock at all. Both bugs are in the same function; they are independent defects with independent fixes. The lock fix is one extra with self._lock: line; the rollback fix is snapshotting and restoring three more attributes.

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🟣 sdk/python/storage/keeper_secrets_manager_storage_gcp_kms/keeper_secrets_manager_storage_gcp_kms/storage_gcp_kms.py:260-266 — Pre-existing dead code in load_config: the inner except at line 266-269 sets decryption_error = True and immediately raise Exception("Failed to parse decrypted config file ..."), so the raise propagates past lines 271-273 to the outer except at line 275. The if json_error and decryption_error: block at lines 271-273 is therefore unreachable, the "may contain JSON format problems" message never surfaces, and the decryption_error local (initialized at 234, mutated at 267, never read on any reachable path) is dead state. The structure predates this PR — only md5→sha256 and the narrowing of the OTHER inner except at line 245 changed in this region — but since the file is open, cheap cleanup: drop the decryption_error local and the unreachable block, or (if the JSON-format message was meant to surface) move the inner raise out of the except.

  Extended reasoning...

  What the bug is

  load_config (storage_gcp_kms.py:215-277) has two sibling inner try/except blocks inside its outer try:

  • try-A (lines 235-246): attempts contents.decode() and json.loads() to detect a plain-JSON config. Its except (line 245, narrowed in this PR to (json.JSONDecodeError, UnicodeDecodeError)) stores the failure in json_error.
  • try-B (lines 260-269): inside the if json_error: branch, attempts json.loads(config_json) on the result of decrypt_buffer. Its except (lines 266-269) sets decryption_error = True, logs, and immediately raises Exception("Failed to parse decrypted config file ...").

  The raise at line 269 escapes try-B and the surrounding if json_error: block, propagating directly to the outer except at line 275, which logs and re-raises. So on every path, lines 271-273 (if json_error and decryption_error: raise Exception("may contain JSON format problems")) are unreachable:

  1. try-B succeeds → decryption_error stays False → condition False, branch skipped.
  2. try-B fails → decryption_error is set to True but the very next statement is a raise → control skips line 271 entirely.

  The decryption_error local at line 234 is therefore dead state — assigned, mutated in one place, never read on any reachable path. The "may contain JSON format problems" message it gates can never surface to the user; the actual exception that reaches the caller is always "Failed to parse decrypted config file" from line 269 (or the original load error from line 225).

  Step-by-step proof

  1. Storage initialized; file on disk is an encrypted blob.
  2. load_config reads contents (encrypted bytes).
  3. Line 236: contents.decode() raises UnicodeDecodeError (or json.loads raises JSONDecodeError).
  4. Line 245 catches it: json_error = err.
  5. if json_error: at line 248 is true. decrypt_buffer runs at line 252. Suppose it returns a string that fails to parse as JSON (or returns "" from the still-swallowing decrypt path).
  6. Line 261: json.loads(config_json) raises.
  7. Line 266 catches it. Line 267 sets decryption_error = True. Line 268 logs. Line 269: raise Exception("Failed to parse decrypted config file ...").
  8. The raise propagates up. Line 271 is never evaluated. Line 275 catches the exception, logs, re-raises.

  The user sees "Failed to parse decrypted config file". They never see "may contain JSON format problems".

  Why this is pre-existing, not introduced

  Pre-PR, the structure was identical — only md5 → sha256 and the narrowing of try-A's except (line 245) from broad Exception to (JSONDecodeError, UnicodeDecodeError) changed in this region. The narrowing of try-A is the right KSM-938-adjacent fix (it stops misclassifying KMS errors as JSON errors, addressed elsewhere in this review), but it does not affect the reachability of try-B's except or the if json_error and decryption_error: block — those reside in a sibling try/except, not nested inside try-A. Pre-PR, with broad except Exception at line 245, the raise at line 269 was also already not caught by that handler, because try-A had already exited before if json_error: ran. The dead code predates this PR.

  Impact

  Zero functional impact. The actual user-facing exception ("Failed to parse decrypted config file") is still raised correctly at line 269 and propagates to the caller. The cost is purely dead code: a dead local (decryption_error), an unreachable branch (lines 271-273), and a never-surfaced error message ("may contain JSON format problems"). Mildly misleading to a future reader — the unreachable branch suggests a code path that does not exist.

  How to fix

  Cheapest cleanup — drop both:

  # remove line 234:      decryption_error = False
  # remove line 267:      decryption_error = True
  # remove lines 271-273: the if json_error and decryption_error block

  Alternatively, if the "may contain JSON format problems" message was actually intended to surface to the user, move the raise out of try-B's except so decryption_error can flow to line 271:

  except Exception as err:
      decryption_error = True
      self.logger.error(f"Failed to parse decrypted config file: {err}")
      # raise removed — let flow continue to line 271

  Either is one to four lines. Given the PR is already touching this function for the md5→sha256 and except-narrowing changes, it's reasonable cleanup while the file is open. Filing as pre_existing since the structural defect was not introduced by this PR.

[claude]

⚠️ Code review skipped — your organization has reached its monthly code review spending cap.

An organization admin can view or raise the cap at claude.ai/admin-settings/claude-code. The cap resets at the start of the next billing period.

Once the cap resets or is raised, push a new commit or reopen this pull request to trigger a review.

[claude]

⚠️ Code review skipped — your organization has reached its monthly code review spending cap.

An organization admin can view or raise the cap at claude.ai/admin-settings/claude-code. The cap resets at the start of the next billing period.

Once the cap resets or is raised, push a new commit or reopen this pull request to trigger a review.

[claude]

⚠️ Code review skipped — your organization has reached its monthly code review spending cap.

An organization admin can view or raise the cap at claude.ai/admin-settings/claude-code. The cap resets at the start of the next billing period.

Once the cap resets or is raised, push a new commit or reopen this pull request to trigger a review.