From f9733654b01d0382df620f2b0950cd62d0e2cc9c Mon Sep 17 00:00:00 2001 From: Simon Woolf Date: Wed, 29 Jul 2026 11:15:58 +0100 Subject: [PATCH] Encrypt annotation data payloads on encrypted channels Annotation had no encrypt() method, so annotation data was published in plaintext even when the channel had a cipher configured, while Message and PresenceMessage were correctly encrypted before serialisation. The REST get() path compounded this by hardcoding cipher=None when building its response handler, so it could not have decrypted an encrypted annotation either. (The realtime receive path already passed the channel's cipher.) Add Annotation.encrypt(), mirroring Message.encrypt(), and call it from both publish paths. Pass the channel's cipher when decoding REST get() responses. Unlike messages, annotations commonly carry no data at all, so encrypt() returns early rather than failing in TypedBuffer.from_obj(). The docstrings claiming annotations are not encrypted because the server needs to parse them for summarisation were correct for the original early-preview API, where aggregation read JSON data payloads. That changed before public release, when aggregation moved to the count field specifically so the server would not have to read payloads. Co-Authored-By: Claude Opus 5 (1M context) --- ably/realtime/annotations.py | 4 +++ ably/rest/annotations.py | 6 +++- ably/types/annotation.py | 38 ++++++++++++++++++++--- test/unit/annotation_test.py | 59 ++++++++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 6 deletions(-) diff --git a/ably/realtime/annotations.py b/ably/realtime/annotations.py index fbbbb755..57400d45 100644 --- a/ably/realtime/annotations.py +++ b/ably/realtime/annotations.py @@ -56,6 +56,10 @@ async def __send_annotation(self, annotation: Annotation, params: dict | None = f'type = {annotation.type}, action = {annotation.action}' ) + # RSAN1c3: encrypt the data payload on an encrypted channel + if self.__channel.cipher: + annotation.encrypt(self.__channel.cipher) + # Convert to wire format (array of annotations) wire_annotation = annotation.as_dict(binary=self.__channel.ably.options.use_binary_protocol) diff --git a/ably/rest/annotations.py b/ably/rest/annotations.py index fc2b29d5..588ffcf3 100644 --- a/ably/rest/annotations.py +++ b/ably/rest/annotations.py @@ -133,6 +133,10 @@ async def __send_annotation(self, annotation: Annotation, params: dict | None = random_id = base64.b64encode(os.urandom(9)).decode('ascii') + ':0' annotation = annotation._copy_with(id=random_id) + # RSAN1c3: encrypt the data payload on an encrypted channel + if self.__channel.cipher: + annotation.encrypt(self.__channel.cipher) + # Convert to wire format request_body = annotation.as_dict(binary=self.__channel.ably.options.use_binary_protocol) @@ -232,7 +236,7 @@ async def get(self, msg_or_serial, params: dict | None = None): path = self.__base_path_for_serial(message_serial) + params_str # Create annotation response handler - annotation_handler = make_annotation_response_handler(cipher=None) + annotation_handler = make_annotation_response_handler(cipher=self.__channel.cipher) # Return paginated result return await PaginatedResult.paginated_query( diff --git a/ably/types/annotation.py b/ably/types/annotation.py index c0926f58..fea91468 100644 --- a/ably/types/annotation.py +++ b/ably/types/annotation.py @@ -2,6 +2,8 @@ from enum import IntEnum from ably.types.mixins import EncodeDataMixin +from ably.types.typedbuffer import TypedBuffer +from ably.util.crypto import CipherData from ably.util.encoding import encode_data from ably.util.helper import to_text @@ -21,8 +23,6 @@ class AnnotationAction(IntEnum): class Annotation(EncodeDataMixin): """ Represents an annotation on a message, such as a reaction or other metadata. - - Annotations are not encrypted as they need to be parsed by the server for summarization. """ def __init__(self, @@ -140,11 +140,35 @@ def id(self): def connection_id(self): return self.__connection_id + def encrypt(self, channel_cipher): + """ + Encrypt the annotation's data payload in place. + + Mirrors Message.encrypt(); call before as_dict() when publishing on an + encrypted channel. + """ + # Annotations commonly carry no data at all, e.g. a reaction that is fully + # described by its name, so there is nothing to encrypt. + if self.__data is None: + return + + if isinstance(self.__data, CipherData): + return + + if isinstance(self.__data, str): + self._encoding_array.append('utf-8') + elif isinstance(self.__data, (dict, list)): + self._encoding_array.append('json') + self._encoding_array.append('utf-8') + + typed_data = TypedBuffer.from_obj(self.__data) + encrypted_data = channel_cipher.encrypt(typed_data.buffer) + self.__data = CipherData(encrypted_data, typed_data.type, + cipher_type=channel_cipher.cipher_type) + def as_dict(self, binary=False): """ Convert annotation to dictionary format for API communication. - - Note: Annotations are not encrypted as they need to be parsed by the server. """ request_body = { 'action': int(self.action) if self.action is not None else None, @@ -171,7 +195,11 @@ def from_encoded(obj, cipher=None, context=None): """ Create an Annotation from an encoded object received from the API. - Note: cipher parameter is accepted for consistency but annotations are not encrypted. + Args: + obj: The encoded annotation + cipher: The channel's cipher, used to decrypt the data payload if it + was published on an encrypted channel + context: Optional decoding context """ action = obj.get('action') serial = obj.get('serial') diff --git a/test/unit/annotation_test.py b/test/unit/annotation_test.py index 947ed04e..2cfb0bef 100644 --- a/test/unit/annotation_test.py +++ b/test/unit/annotation_test.py @@ -8,6 +8,7 @@ - RSAN1c1/RSAN2a: explicit action setting in publish/delete - TAN3: from_encoded / from_encoded_array decoding - TAN2i: serial-based equality +- RSAN1c3: encryption of the data payload on an encrypted channel """ import base64 @@ -17,6 +18,7 @@ from ably.rest.annotations import construct_validate_annotation, serial_from_msg_or_serial from ably.types.annotation import Annotation, AnnotationAction from ably.types.message import Message +from ably.util.crypto import generate_random_key, get_cipher from ably.util.exceptions import AblyException # --- RSAN1a3: type validation --- @@ -317,3 +319,60 @@ def test_from_encoded_array(): assert annotations[0].action == AnnotationAction.ANNOTATION_CREATE assert annotations[1].name == '👎' assert annotations[1].action == AnnotationAction.ANNOTATION_DELETE + + +# --- encryption of the data payload --- + +def _cipher(): + return get_cipher({'key': generate_random_key(256)}) + + +@pytest.mark.parametrize('binary', [False, True]) +@pytest.mark.parametrize('payload', [ + 'secret text', + {'secret': 'value'}, + ['secret', 1], + bytearray(b'secret bytes'), +]) +def test_encrypt_data_round_trips(payload, binary): + """encrypt() must encrypt the data payload, and from_encoded() must decrypt it""" + cipher = _cipher() + annotation = Annotation(type='reaction:distinct.v1', name='👍', data=payload) + annotation.encrypt(cipher) + + d = annotation.as_dict(binary=binary) + assert 'cipher+aes-256-cbc' in d['encoding'] + assert 'secret' not in str(d['data']) + + decoded = Annotation.from_encoded(d, cipher=cipher) + assert decoded.data == payload + + +def test_encrypt_without_data_is_a_noop(): + """Annotations commonly carry no data, which must not error""" + annotation = Annotation(type='reaction:distinct.v1', name='👍') + annotation.encrypt(_cipher()) + assert annotation.data is None + assert 'data' not in annotation.as_dict() + + +def test_encrypt_is_idempotent(): + """Already-encrypted data must not be encrypted a second time""" + cipher = _cipher() + annotation = Annotation(type='reaction:distinct.v1', data='secret text') + annotation.encrypt(cipher) + once = annotation.as_dict()['data'] + annotation.encrypt(cipher) + assert annotation.as_dict()['data'] == once + + +def test_from_encoded_without_cipher_leaves_residual_encoding(): + """Decoding encrypted data with no cipher must leave the transform in `encoding`""" + cipher = _cipher() + annotation = Annotation(type='reaction:distinct.v1', data='secret text') + annotation.encrypt(cipher) + d = annotation.as_dict() + + decoded = Annotation.from_encoded(d) + assert 'cipher+aes-256-cbc' in decoded.encoding + assert decoded.data != 'secret text'