• 
      

    Add a class for handling secrets encoding/decoding.

    Review Request #15182 — Created July 21, 2026 and updated

    Information

    cryptozoology
    master

    Reviewers

    When managing stored secrets (such as auth-related state or other
    sensitive data), the current best practice is to:

    1. Generate a one-time-use key (a DEK, or Data Encryption Key)
    2. Encrypt the plaintext
    3. Key-wrap (effectively encrypt) the DEK using a parent KEK (Key
      Encryption Key), identified by a KID (key ID)
    4. Store the resulting encrypted DEK, the KEK's KID, the encryption and
      keywrap algorithms, and a version as an envelope header, with the
      ciphertext appended.

    This change implements this through a Secret class, which manages a
    plaintext and allows for encoding and decoding. It will generate a DEK
    (unless one is available -- provided when decoding or for unit tests).

    Decoding takes a KEK resolver that returns the key matching the ID.
    This keeps it separate from any key management mechanism (though once
    that's built, this will cleanly integrate).

    Secret is invalidatable, which means that it will automatically clear
    its internal state when it falls out of scope or when used as a context
    manager and the context closes. This helps avoid situations where the
    resulting plaintext or DEK may be lingering somewhere accidentally.
    Callers should decode a Secret using with Secret.from_encoded()
    and run all operations that need access to the plaintext within that
    context.

    Unit tests pass.

    Summary ID
    Add a class for handling secrets encoding/decoding.
    When managing stored secrets (such as auth-related state or other sensitive data), the current best practice is to: 1. Generate a one-time-use key (a DEK, or Data Encryption Key) 2. Encrypt the plaintext 3. Key-wrap (effectively encrypt) the DEK using a parent KEK (Key Encryption Key), identified by a KID (key ID) 4. Store the resulting encrypted DEK, the KEK's KID, the encryption and keywrap algorithms, and a version as an envelope header, with the ciphertext appended. This change implements this through a `Secret` class, which manages a plaintext and allows for encoding and decoding. It will generate a DEK (unless one is available -- provided when decoding or for unit tests). Decoding takes a KEK resolver that returns the key matching the ID. This keeps it separate from any key management mechanism (though once that's built, this will cleanly integrate). `Secret` is invalidatable, which means that it will automatically clear its internal state when it falls out of scope or when used as a context manager and the context closes. This helps avoid situations where the resulting plaintext or DEK may be lingering somewhere accidentally. Callers should decode a `Secret` using `with Secret.from_encoded()` and run all operations that need access to the plaintext within that context.
    0a4fe505d9b1aeffc4a1b9a8ec6905299f33136c
    Description From Last Updated

    This should probably be called _KEY_CLASSES. We should also type as Final[...] and set it to a tuple instead of …

    david david

    Even though this is typed as Final, it's assigned as a mutable set. Can we use frozenset here?

    david david

    If the secret has been invalidated, we'll silently encrypt nothing. Can we start this method with: if not self.is_valid(): raise …

    david david

    This will give a misleading error in the case that the version is present but malformed. Can we break this …

    david david

    This should probably raise SecretDecodeError(...) from None so that Python doesn't chain underlying library detail in the traceback/logs. Same below …

    david david

    This is read from the envelope and used verbatim to decrypt without doing any kind of authentication. This means if …

    david david

    This validates against every class, but the actual unwrap goes through kek.unwrap_key(), so it might pass validation here even if …

    david david

    Using this as a context manager clears its state on exit. This means that calling this method will destroy the …

    david david

    I think we should be a little more limited in what we catch so it's just what can come from …

    david david

    This should probably be formatting in message instead of e (like you do above in the handler after unwrap_key())

    david david

    In from_encoded, you add in the version field (_RESERVED_FIELDS | {version_field}). We should do the same here so that extra_fields={'encv': …

    david david

    The value is quoted, but the key is not (and not validated). A ; character in a key could cause …

    david david

    This doesn't have "Version Added"

    david david

    We should probably add and self.dek.is_valid() so an invalid DEK doesn't cause a confusing failure later.

    david david

    This doesn't have "Version Added"

    david david

    This is repeated verbatim 12 times. Can we create a helper to make the resolver?

    david david

    This text has changed across python versions. Can we just assert on message.startswith('Corrupted ciphertext:')?

    david david
    Checks run (2 succeeded)
    flake8 passed.
    JSHint passed.
    david
    1. 
        
    2. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
      Show all issues

      This should probably be called _KEY_CLASSES.

      We should also type as Final[...] and set it to a tuple instead of a mutable list.

      1. Placeholder for the registry change, but I'll fix it up.

    3. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
       
       
      Show all issues

      Even though this is typed as Final, it's assigned as a mutable set. Can we use frozenset here?

    4. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
      Show all issues

      If the secret has been invalidated, we'll silently encrypt nothing. Can we start this method with:

      if not self.is_valid():
          raise ValueError(
              'This secret has been invalidated and cannot be encoded.')
      

      Would be good to have a test for calling encode after invalidation.

      1. This was probably meant for .encode(). Fixing it there.

    5. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
       
      Show all issues

      This will give a misleading error in the case that the version is present but malformed. Can we break this apart?

      try:
          version_str = fields[version_field]
      except KeyError:
          raise SecretDecodeError('Envelope version field is missing.') from None`
      
      try:
          version = int(version_str)
      except ValueError:
          raise SecretDecodeError(
              f'Invalid envelope version {version_str!r}.'
          ) from None
      
    6. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
       
       
      Show all issues

      This should probably raise SecretDecodeError(...) from None so that Python doesn't chain underlying library detail in the traceback/logs. Same below wherever we catch one exception and raise another.

    7. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
      Show all issues

      This is read from the envelope and used verbatim to decrypt without doing any kind of authentication. This means if it was rewritten from alg=AES-256-GCM to AES-256-CFB8, it would disable authentication on the decryption.

      I think there are two things we should do to mitigate this:

      1. Bind the header as AAD (pass the envelope prefix into encrypt/decrypt as associated data). This would also make it so extra_fields would be authenticated (instead of freely rewritable).
      2. Restrict from_encoded to AEAD algorithms (perhaps allowing an escape hatch to CFB8 if we need it).
      1. You're right, and how did I skip past this... I had it in my reference implementation. Fixing the AAD.

    8. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
       
       
       
       
       
       
       
       
       
      Show all issues

      This validates against every class, but the actual unwrap goes through kek.unwrap_key(), so it might pass validation here even if the underlying KEK can't do it. How about we just validate against kek directly?

      kek = kek_resolver(kid)
      
      if not kek.supports_keywrap_alg(wrap_alg):
          raise SecretDecodeError(...)
      
    9. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
      Show all issues

      Using this as a context manager clears its state on exit. This means that calling this method will destroy the kek that gets passed in. For some cases this may not matter, but a KEK seems like it could be a long-lived object that a caller would expect to be able to hand out repeatedly:

      kek = load_kek()
      
      with Secret.from_encoded(enc1, kek_resolver=lambda kid: kek) as s1:
          ...
      
      with Secret.from_encoded(enc2, kek_resolver=lambda kid: kek) as s2:
          # this fails
      

      Taking ownership of the passed-in KEK seems highly fragile, so I think we'd do better to just have the try/catch without the context manager use here.

      Would be good to have a test for this situation too.

      1. The intent was for key_resolver to always return a new instance so it can be in memory for only a brief period, to be immediately invalidated. Objects shouldn't hold onto keys for long periods at a time. Let me give this some thought...

      2. Okay, because the KEK resolver is meant to be a bridge to a key storage mechanism (and ultimately will be tied directly to the key storage backends), it's correct to expect a new instance and to then invalidate. If we wanted to support a use case of a caller providing an explicit KEK (which would require validation against the KID), then that should be a separate kek parameter rather than a dynamic lookup. But I don't think we want that at this point, given the intent of the design. So current behavior is correct.

    10. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
      Show all issues

      I think we should be a little more limited in what we catch so it's just what can come from the crypto later: KeyUnwrapError, UnsupportedAlgorithmLayer, CryptozoologyError, and ValueError. As-is this can swallow TypeError or AttributeErrors that come from genuine bugs.

      Same with the except for dek.decrypt() just below.

    11. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
      Show all issues

      This should probably be formatting in message instead of e (like you do above in the handler after unwrap_key())

    12. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
      Show all issues

      In from_encoded, you add in the version field (_RESERVED_FIELDS | {version_field}). We should do the same here so that extra_fields={'encv': '...'} doesn't silently create an unreadable secret.

      Might be worth a test that passes version_field in the extra fields to verify.

    13. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
       
       
       
      Show all issues

      The value is quoted, but the key is not (and not validated). A ; character in a key could cause envelope field injection. Given that extra_fields is meant to be a general-purpose mapping, we should make sure it's safe by default.

      Would need a corresponding change in decode

      We might also want to call quote() with safe=''. Right now / characters aren't escaped, which shouldn't be a problem currently, but I'd prefer to not have that assumption baked in.

    14. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
      Show all issues

      This doesn't have "Version Added"

    15. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
      Show all issues

      We should probably add and self.dek.is_valid() so an invalid DEK doesn't cause a confusing failure later.

    16. cryptozoology/operations/secrets.py (Diff revision 1)
       
       
      Show all issues

      This doesn't have "Version Added"

    17. cryptozoology/operations/tests/test_secret.py (Diff revision 1)
       
       
       
       
       
       
       
      Show all issues

      This is repeated verbatim 12 times. Can we create a helper to make the resolver?

    18. cryptozoology/operations/tests/test_secret.py (Diff revision 1)
       
       
       
       
       
       
      Show all issues

      This text has changed across python versions. Can we just assert on message.startswith('Corrupted ciphertext:')?

    19.