diff --git a/docs/operations.md b/docs/operations.md index f96a036..b88ec68 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -316,6 +316,18 @@ there isn't one to run. Changing `STURNUS_MASTER_KEY` today makes every previously wrapped data key permanently unreadable (see below), it does not rotate anything. +**It also wraps things that are not recordings, and one of them outlives +every recording.** Three kinds of material are sealed under it beyond the +audio: an export destination's credential and a guild's OAuth client +secret, both in the database and both bound to their guild and purpose; +and — since export targets could store a rendered protocol — the +protocol artefacts themselves in the object bucket. Those artefacts do +*not* use a session data key. Each carries a data key of its own, wrapped +under the master key inside the object, because a protocol is deliberately +not subject to `audio_retention_days` and a key that the retention sweep +ends the life of would end the protocol's life with it. §6 states that +rule from the retention side. + **Losing a master key destroys every recording it wrapped.** This is the single most consequential operational fact about this system, so it is stated here plainly: audio is only ever stored encrypted, and only the @@ -1298,6 +1310,46 @@ this sweep is the only thing in the system that ends a recording's life. partial failure is retried on the next sweep rather than recorded as done. +**The sweep does not touch a session's stored protocol, and it must not +start.** A guild publishing to a `markdown` or `html` export target gets +an artefact in the same bucket the recordings are in +(`{prefix}/{session_id}/{target_id}.md`), and nothing in this system ever +deletes it: not this sweep, not the console, not a re-export, which +replaces the object at the same address rather than adding one. That is +the intended rule and not an omission. `audio_retention_days` governs the +*recording*; the record of the meeting deliberately outlives it, which is +why a transcript endpoint answers `200` with `audio_available: false` +rather than `404` once the audio is gone. A protocol swept because its +recording expired would be a meeting's minutes deleted on the strength of +a window that was never about them. + +Two consequences an operator has to plan for. **These objects accumulate** +— one per session per object-store destination, tens of kilobytes each, +and the bucket lifecycle rule that backstops the recordings must not be +written broadly enough to catch them. And **their retention is a policy +question this system does not answer**: if a deployment needs stored +protocols to expire, that is a rule somebody has to state, and there is no +setting for it today. + +They are encrypted, and **not under the recording's key**. Each artefact +carries a data key of its own, generated when the artefact is written and +wrapped under `STURNUS_MASTER_KEY` inside the object itself, bound to the +guild it belongs to. So an artefact opens with the master key and the +guild alone: it needs no row to have survived, and nothing the sweep +destroys. Sealing one under the session data key instead would have made +every stored export unreadable the day its recording's window closed — +silently, and noticed only by whoever next opened an old document. + +**Protocols written before v0.17.0 are plaintext in that bucket.** The +release that introduced export targets stored them unencrypted; nothing +sweeps them, so they are still there. They are still served — refusing +them would turn a link somebody already holds into a 404 over bytes that +are already written — and each read logs `session.export_unsealed` at +WARNING, which is how you watch that corpus drain. Re-publishing a session +replaces its object with a sealed one at the same address; there is no +bulk migration, deliberately, because it would need to re-render every +protocol ever published. + Because recordings outlive their transcription by weeks, not minutes, the retention period is not merely an implementation detail — **it belongs in the privacy policy shown to participants** (the `policy_url` document), diff --git a/src/sturnus/application/export_formats.py b/src/sturnus/application/export_formats.py index 6fca77d..741ad29 100644 --- a/src/sturnus/application/export_formats.py +++ b/src/sturnus/application/export_formats.py @@ -94,13 +94,20 @@ class RenderRequest: class ExportFormat: """One format: how a protocol is rendered, and what carries it away. - `media_type` and `file_extension` are read only by the object-store - family, which has to name the bytes it stores and the object it stores - them in. They are on every entry regardless, because "what kind of - document is this" is a property of the format rather than of the sink - that happens to take it -- `session_document.provider` records the - format name, and the console route that serves an artefact back reads - its media type from here rather than guessing from the key. + `file_extension` names the object an object-store destination stores + its bytes in; `media_type` names what those bytes *are* to a reader. + They are on every entry regardless, because "what kind of document is + this" is a property of the format rather than of the sink that happens + to take it -- `session_document.provider` records the format name, and + the console route that serves an artefact back reads its media type + from here rather than guessing from the key. + + **The stored object does not carry the media type, and that is + deliberate.** An object-store artefact is a sealed envelope + (`sturnus.infrastructure.documents.artefacts`), so `text/markdown` is + true of the document and false of the object; the entry below is the + one place that answer lives, and it reaches a browser through the + console route rather than through S3 metadata. """ name: str diff --git a/src/sturnus/application/exporting.py b/src/sturnus/application/exporting.py index aef3ffe..b792e67 100644 --- a/src/sturnus/application/exporting.py +++ b/src/sturnus/application/exporting.py @@ -103,6 +103,17 @@ class Destination: configured target and the guild's `document_provider` setting for the legacy one, so nothing about what an existing guild writes to that column changes. + + `guild_id` is carried rather than looked up because a sink may need to + say *whose* artefact this is at the moment it writes one: an + object-store destination seals its bytes under a key bound to the + guild and the purpose (`sturnus.infrastructure.crypto.secret_context`), + and a sink that had to ask a database which guild it was serving would + be a sink with a database. It is the same guild for every destination + of one publish -- they are that guild's destinations -- which is + precisely why it belongs on the destination rather than being threaded + through `publish_session` as a second parameter that could disagree + with it. """ session_id: int @@ -110,6 +121,7 @@ class Destination: format: ExportFormat target: str provider: str + guild_id: int @dataclass(frozen=True, slots=True) @@ -290,6 +302,11 @@ def destinations_for( format=entry, target=target.target, provider=entry.name, + # The row's own guild, not one passed in beside it: this + # value ends up in the associated data that seals an + # object-store artefact, and a mismatch between it and + # the row would be an artefact nobody can open again. + guild_id=target.guild_id, ) ) if chosen: diff --git a/src/sturnus/application/worker.py b/src/sturnus/application/worker.py index ddd788b..7c3708f 100644 --- a/src/sturnus/application/worker.py +++ b/src/sturnus/application/worker.py @@ -498,6 +498,7 @@ async def _legacy_destination( # this is what `session.document_provider` has always been written # with, and nothing about an existing guild's rows changes here. provider=provider, + guild_id=guild, ) diff --git a/src/sturnus/console/adapters.py b/src/sturnus/console/adapters.py index 8852505..a3f3443 100644 --- a/src/sturnus/console/adapters.py +++ b/src/sturnus/console/adapters.py @@ -2504,10 +2504,16 @@ async def _read( if target_id is not None: statement = statement.where(SessionDocumentRow.target_id == target_id) async with self._session_factory() as db: - exists = await db.scalar( - select(select(SessionRow.id).where(SessionRow.id == session_id).exists()) + # The guild, from the statement that was already establishing + # that the session exists. A read model carries it because an + # object-store artefact is sealed under a key bound to it + # (`sturnus.domain.exports.SessionDocument`), and asking a + # second time would be a second query for a column this one + # is already looking at. + guild_id = await db.scalar( + select(SessionRow.guild_id).where(SessionRow.id == session_id) ) - if not exists: + if guild_id is None: return None rows = await db.scalars( # Publication order, so the list reads as the history it @@ -2518,6 +2524,7 @@ async def _read( return tuple( SessionDocument( session_id=row.session_id, + guild_id=guild_id, target_id=row.target_id, provider=row.provider, document_id=row.document_id, diff --git a/src/sturnus/console/ports.py b/src/sturnus/console/ports.py index 9fc7e34..4e49c81 100644 --- a/src/sturnus/console/ports.py +++ b/src/sturnus/console/ports.py @@ -448,14 +448,28 @@ async def document_of(self, session_id: int, target_id: int) -> SessionDocument class DocumentArtefacts(Protocol): - """Where a stored protocol's bytes are read from. `S3DocumentStore`. + """Where a stored protocol's bytes are read from. `SealedArtefacts`. `KeyError` for an object that is not there, which is an ordinary outcome rather than a fault: a re-export can move nothing, but a destination removed from the bucket by hand leaves the row behind. + `sturnus.domain.errors.UnreadableArtefact` for one that is there and + does not open, which is not ordinary -- see that class for why the two + are different exceptions rather than one. + + `guild_id` is what the artefact's own key is bound to, so it is the + caller's to supply and it comes off the `SessionDocument` row rather + than out of the object. An envelope carrying the guild it was filed + under would authenticate just as happily after being moved to another + guild's, which is the move this binding exists to fail. + + The adapter behind this holds the master key and this port says + nothing about it: the console decides *who* may read a protocol, and + what it takes to turn an object into bytes is not a question a route + should be able to ask. """ - async def get(self, key: str) -> bytes: ... + async def get(self, key: str, *, guild_id: int) -> bytes: ... class AdminDirectory(Protocol): diff --git a/src/sturnus/console/routes_documents.py b/src/sturnus/console/routes_documents.py index 83ad949..9511550 100644 --- a/src/sturnus/console/routes_documents.py +++ b/src/sturnus/console/routes_documents.py @@ -28,6 +28,15 @@ distinct one would confirm which sessions the system holds and where a guild publishes, to somebody with no business knowing either. +**The bytes in the bucket are sealed, and this is where they are opened.** +The rule above is what this process enforces; the encryption is a property +of the object, and the two answer different questions about a bucket +somebody has a copy of. Which key an artefact is sealed under -- its own, +never the recording's -- is +`sturnus.infrastructure.documents.artefacts`' decision and its docstring's +argument; all that reaches this module is a `guild_id` off the row and a +second way for the read to fail. + **An Outline document is not served here.** Its bytes live in Outline, and the listing carries its URL so the console can link straight out. Only the formats whose sink family is the object store have anything for this route @@ -44,6 +53,7 @@ from sturnus.application.export_formats import OBJECT_STORE_SINK, format_named from sturnus.console.ports import DocumentArtefacts, SessionDocumentDirectory +from sturnus.domain.errors import UnreadableArtefact from sturnus.domain.exports import SessionDocument from sturnus.observability.events import Event, log_event @@ -171,7 +181,9 @@ async def read_document(request: web.Request) -> web.StreamResponse: # media type to `str | None` for a case that cannot happen. assert entry is not None try: - body = await request.app[DOCUMENT_ARTEFACTS].get(document.document_id) + body = await request.app[DOCUMENT_ARTEFACTS].get( + document.document_id, guild_id=document.guild_id + ) except KeyError: # The row outlived its object. Nothing is broken, so this is a 404 # and not a 500 -- the same reading the audio route gives a @@ -187,6 +199,26 @@ async def read_document(request: web.Request) -> web.StreamResponse: reason="artefact_erased", ) raise _not_found() from None + except UnreadableArtefact: + # The object is there and does not open: a wrong master key, an + # envelope sealed under a guild this row does not name, a body + # edited in the bucket. The reader gets the same 404 -- there is + # nothing here to serve them either way -- and the log gets a + # different `reason`, because "the sweep took it" and "it failed + # to authenticate" are different mornings for whoever is on call. + # The exception itself is not logged: it is raised from an + # `InvalidTag`, whose only useful content is the fact of it. + log_event( + log, + logging.ERROR, + Event.CONSOLE_DOCUMENT_REFUSED, + "A protocol's object is in the store and did not open", + session_id=session_id, + target_id=target_id, + requested_by=caller, + reason="artefact_unreadable", + ) + raise _not_found() from None log_event( log, diff --git a/src/sturnus/domain/errors.py b/src/sturnus/domain/errors.py index b10dc38..0179c87 100644 --- a/src/sturnus/domain/errors.py +++ b/src/sturnus/domain/errors.py @@ -10,6 +10,11 @@ both are stdlib-only by construction so that every layer, `domain` included, can raise them. +`UnreadableArtefact` is here for the same reason in the other direction: +it is raised by an adapter in `sturnus.infrastructure` and caught by a +route in `sturnus.console`, and a route reaching into the adapter package +for an exception type would be the route knowing which adapter it has. + Sturnus records people talking. Spec 12.4 -- "Neither audio data nor transcript content appears in logs" -- is the standard the pod logs are held to, and `sturnus.infrastructure.observability` holds Sentry to at least the @@ -85,3 +90,28 @@ class CorruptRecording(Exception): from two modules now; see that class's docstring for what the claim costs to make honestly. """ + + +class UnreadableArtefact(Exception): + """A stored protocol is there, and these are not bytes to serve. + + Raised where a sealed export artefact fails to open: a wrong master + key, an envelope that does not authenticate under the guild it was + found filed against, a body somebody edited in the bucket. Every one + of those is the same answer for the person asking -- there is no + document here -- and the console gives it the same 404 it gives an + artefact the object store no longer has. + + **It is not `KeyError`, and that distinction is the reason the class + exists.** An object that is missing is an ordinary event with an + ordinary cause; an object that is present and will not open is + somebody's meeting failing to authenticate, which is the one outcome + an operator has to be able to find in a log. Folding it into + "artefact erased" would report an integrity failure as a tidy-up. + + A sibling of `CorruptRecording` rather than the same class. That one + is about the stored *audio* format and is raised by two readers of it; + this is about the artefact envelope, which is a different format with + a different lifetime. Naming both "corrupt recording" would make the + one search an operator runs return the other's failures. + """ diff --git a/src/sturnus/domain/exports.py b/src/sturnus/domain/exports.py index 62f19a6..dd51266 100644 --- a/src/sturnus/domain/exports.py +++ b/src/sturnus/domain/exports.py @@ -75,9 +75,19 @@ class SessionDocument: publishing here", not "forget what was published": the document still exists in the other system, and the link is what somebody follows when they go looking for last quarter's minutes. + + `guild_id` is the session's guild, joined rather than stored on the + row: `session_document` has no such column and does not need one, + because a document's guild is its session's and cannot be anything + else. It is on the read model because an object-store artefact is + sealed under a key bound to that guild, and the reader has to supply + that binding from its own context rather than from the object -- an + envelope that carried the guild it was filed under would authenticate + just as happily after being moved. """ session_id: int + guild_id: int target_id: int | None provider: str document_id: str diff --git a/src/sturnus/entrypoints/api.py b/src/sturnus/entrypoints/api.py index 46c81e4..1f37c8d 100644 --- a/src/sturnus/entrypoints/api.py +++ b/src/sturnus/entrypoints/api.py @@ -85,6 +85,7 @@ from sturnus.infrastructure.db.models import GuildOAuthClient as GuildOAuthClientRow from sturnus.infrastructure.db.preferences import PreferenceStore from sturnus.infrastructure.db.setup_intents import SetupIntentStore +from sturnus.infrastructure.documents.artefacts import SealedArtefacts from sturnus.infrastructure.documents.outline_oauth import OutlineOAuth from sturnus.infrastructure.objectstore import S3AudioStore, S3DocumentStore from sturnus.infrastructure.observability import init_sentry @@ -336,11 +337,20 @@ def now() -> datetime: # no second class to say so. exports=ExportTargetStore(session_factory, keys), documents=ConsoleSessionDocuments(session_factory), - artefacts=S3DocumentStore( - settings.s3_endpoint, - settings.s3_bucket, - settings.s3_access_key.get_secret_value(), - settings.s3_secret_key.get_secret_value(), + # `keys` again: a stored protocol is sealed under a data key of + # its own, wrapped under this process's master key and bound to + # the guild. That this process already holds the master key -- + # it decrypts audio on the way to the browser -- is why serving + # an artefact needs no credential the chart does not already give + # it (`charts/sturnus/templates/_helpers.tpl`). + artefacts=SealedArtefacts( + S3DocumentStore( + settings.s3_endpoint, + settings.s3_bucket, + settings.s3_access_key.get_secret_value(), + settings.s3_secret_key.get_secret_value(), + ), + keys, ), oauth_clients=ConsoleGuildOAuthClients(oauth_clients, admins), setup=ConsoleGuildSetup(session_factory, admins, SetupIntentStore(session_factory)), diff --git a/src/sturnus/entrypoints/worker.py b/src/sturnus/entrypoints/worker.py index f5c24ba..757bf4b 100644 --- a/src/sturnus/entrypoints/worker.py +++ b/src/sturnus/entrypoints/worker.py @@ -99,6 +99,7 @@ SessionRepository, ) from sturnus.infrastructure.db.session_documents import SessionDocumentStore +from sturnus.infrastructure.documents.artefacts import SealedArtefacts from sturnus.infrastructure.documents.outline import OutlineSink from sturnus.infrastructure.documents.sinks import DocumentSinks from sturnus.infrastructure.health import ReadinessState, start_health_server @@ -619,12 +620,19 @@ async def _run() -> None: # The same bucket the recordings are in, through a class that reads and # writes whole small objects rather than streaming large encrypted ones # -- see `sturnus.infrastructure.objectstore`, which keeps the two - # apart on purpose. - document_objects = S3DocumentStore( - settings.s3_endpoint, - settings.s3_bucket, - settings.s3_access_key.get_secret_value(), - settings.s3_secret_key.get_secret_value(), + # apart on purpose -- and sealed on the way in. `keys` again, the same + # one the export targets' credentials are wrapped with: one decode of + # the master key in this process. An artefact is *not* sealed under + # the recording's data key, and `sturnus.infrastructure.documents. + # artefacts` is where that decision is argued. + document_objects = SealedArtefacts( + S3DocumentStore( + settings.s3_endpoint, + settings.s3_bucket, + settings.s3_access_key.get_secret_value(), + settings.s3_secret_key.get_secret_value(), + ), + keys, ) # Tracing is applied here, on the way into `process_one`, and nowhere diff --git a/src/sturnus/infrastructure/crypto.py b/src/sturnus/infrastructure/crypto.py index ea79330..703b9eb 100644 --- a/src/sturnus/infrastructure/crypto.py +++ b/src/sturnus/infrastructure/crypto.py @@ -1,4 +1,4 @@ -"""Envelope encryption for recorded audio (Spec 12.1). +"""Envelope encryption for recorded audio, and for what outlives it (Spec 12.1). A fresh data key is generated per session and encrypted with the master key from the environment; only the wrapped form is stored, alongside the id of @@ -19,6 +19,16 @@ anything. Both need the arithmetic, and the alternative to exporting it is a second copy of the format written down somewhere else -- which is how a format ends up with two definitions that disagree. + +**There are two formats here, and the second one exists because of a +lifetime rather than a size.** `encrypt_file` is the chunked recording +format above; `seal_artefact` is one small object sealed in one piece, +carrying the wrapped key that opens it. A recording keeps its wrapped key +in the row that owns it, which is right for something the retention sweep +deletes; a rendered protocol has no such row and must not acquire one, +because it is written once and read back long after that sweep has run. +See `seal_artefact` for the full argument and for why the associated data +it takes is a parameter rather than a field of the envelope. """ from __future__ import annotations @@ -37,6 +47,13 @@ #: The framing, as read by anything that is not `decrypt_file`. MAGIC = b"STRN\x01" +#: The *other* format in the same family: one small object sealed in one +#: piece, carrying the wrapped key that opens it. `seal_artefact` says why +#: it is a second format rather than a second caller of `encrypt_file`. +#: Same four-byte family, next version byte, so a reader holding an object +#: and no context can still say what it is looking at -- and so neither +#: reader silently accepts the other's bytes. +ARTEFACT_MAGIC = b"STRN\x02" FILE_PREFIX_BYTES = 8 LENGTH_BYTES = 4 #: AES-GCM appends a 16-byte authentication tag, so a sealed chunk is @@ -51,6 +68,16 @@ #: ciphertext offset of chunk `n` a multiplication rather than a scan. FRAME_BYTES = LENGTH_BYTES + CHUNK_SIZE + TAG_BYTES +#: How many bytes the artefact envelope spends saying how long its wrapped +#: key is. Two, because a wrapped 32-byte key is sixty bytes today and the +#: only reason the length is written down at all is so that a future +#: wrapping scheme with a different size can be read by this same parser. +_WRAPPED_LENGTH_BYTES = 2 +#: The nonce sealing an artefact's body. Twelve random bytes rather than a +#: prefix and a counter, because there is exactly one seal per artefact -- +#: see `seal_artefact` on why a protocol is not chunked. +_SEAL_NONCE_BYTES = 12 + @dataclass(frozen=True) class DataKey: @@ -180,3 +207,98 @@ def decrypt_file(source: Path, target: Path, data_key: bytes) -> None: raise ValueError("truncated chunk") dst.write(aead.decrypt(nonce(prefix, counter), sealed, None)) counter += 1 + + +def is_sealed_artefact(blob: bytes) -> bool: + """Whether these bytes are an artefact this module sealed. + + Read before `open_artefact` by anything that also has to serve the + plaintext artefacts written before this format existed. The magic is + five bytes ending in a control character, so a Markdown or HTML + document cannot begin with it by accident. + """ + return blob.startswith(ARTEFACT_MAGIC) + + +def seal_artefact(plaintext: bytes, keys: KeyWrapper, aad: bytes) -> bytes: + """Seals one small artefact under a data key of its own, and encloses it. + + **Why this is a second format and not a second caller of + `encrypt_file`.** That one chunks, because a recording runs to + hundreds of megabytes and AES-GCM in a single call would mean holding + all of it in memory; and it stores no key, because a recording's + wrapped data key is a column on the job that owns it. A rendered + protocol is tens of kilobytes -- one seal, no chunking -- and it has + no such column, deliberately: it is written once and read back years + later, long after the job that produced the recording has had its + audio swept and its retention stamped. + + So the key travels with the object. A fresh data key per artefact, + wrapped under the master key and written into the envelope, gives an + object that is complete on its own: restore the bucket and the master + key, and every protocol in it still opens, with nothing in the + database needing to have survived alongside. It is envelope + encryption exactly as the recordings use it, with the wrapped key in + the object rather than in a row -- against somebody holding the + bucket that is the same key wrapped under the same master key, and + against somebody holding only the database it is strictly less. + + **`aad` is what stops the envelope being portable**, and it must come + from the reader's own context rather than from the object -- which is + why it is a parameter here and not a field of the envelope. See + `secret_context`: bound to the guild and to the purpose, an artefact + copied onto another guild's key fails to authenticate instead of + handing that guild somebody else's meeting. + + The body itself is sealed with no associated data. Its key is used + once, for these bytes, and exists nowhere else; what is relocatable + here is the wrapped key, and that is what carries the binding. + """ + data_key = keys.new_data_key(aad) + seal_nonce = os.urandom(_SEAL_NONCE_BYTES) + return b"".join( + ( + ARTEFACT_MAGIC, + struct.pack(">H", len(data_key.wrapped)), + data_key.wrapped, + seal_nonce, + AESGCM(data_key.plaintext).encrypt(seal_nonce, plaintext, None), + ) + ) + + +def open_artefact(sealed: bytes, keys: KeyWrapper, aad: bytes) -> bytes: + """The bytes `seal_artefact` sealed, or a refusal. + + `ValueError` for anything that is not this envelope -- a wrong magic, + a truncation, a length prefix claiming more key than the object holds + -- raised before the master key is asked to unwrap anything. Every + field read out of the object below is a field somebody who can write + to the bucket chooses, so each one is checked against what is + actually there rather than trusted to be honest. + + `InvalidTag` for an envelope that is this format and does not + authenticate: a wrong master key, a wrong `aad`, or a body somebody + edited. The caller has one answer for all three -- these are not + bytes it may serve -- and telling them apart at this level would only + describe the attack back to whoever mounted it. + """ + if not is_sealed_artefact(sealed): + raise ValueError("not a sturnus sealed artefact") + at = len(ARTEFACT_MAGIC) + header = sealed[at : at + _WRAPPED_LENGTH_BYTES] + if len(header) != _WRAPPED_LENGTH_BYTES: + raise ValueError("truncated artefact header") + (wrapped_bytes,) = struct.unpack(">H", header) + at += _WRAPPED_LENGTH_BYTES + wrapped = sealed[at : at + wrapped_bytes] + if len(wrapped) != wrapped_bytes: + raise ValueError("truncated wrapped key") + at += wrapped_bytes + seal_nonce = sealed[at : at + _SEAL_NONCE_BYTES] + if len(seal_nonce) != _SEAL_NONCE_BYTES: + raise ValueError("truncated artefact nonce") + body = sealed[at + _SEAL_NONCE_BYTES :] + if len(body) < TAG_BYTES: + raise ValueError("truncated artefact body") + return AESGCM(keys.unwrap(wrapped, aad)).decrypt(seal_nonce, body, None) diff --git a/src/sturnus/infrastructure/db/session_documents.py b/src/sturnus/infrastructure/db/session_documents.py index d9e2ca1..b1b6355 100644 --- a/src/sturnus/infrastructure/db/session_documents.py +++ b/src/sturnus/infrastructure/db/session_documents.py @@ -25,6 +25,7 @@ from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from sturnus.domain.exports import SessionDocument +from sturnus.infrastructure.db.models import Session as SessionRow from sturnus.infrastructure.db.models import SessionDocument as SessionDocumentRow @@ -82,21 +83,29 @@ async def for_session(self, session_id: int) -> Sequence[SessionDocument]: Publication order, so the list reads as the history it is. Ties break on id, so two reads of an unchanged session agree. + + Joined to `session` for the guild, which the read model carries + and this table does not store -- see + `sturnus.domain.exports.SessionDocument`. An inner join, because a + `session_document` row without its session cannot exist: + `session_id` is `ON DELETE CASCADE`. """ async with self._session_factory() as session: - rows = await session.scalars( - select(SessionDocumentRow) + rows = await session.execute( + select(SessionDocumentRow, SessionRow.guild_id) + .join(SessionRow, SessionRow.id == SessionDocumentRow.session_id) .where(SessionDocumentRow.session_id == session_id) .order_by(SessionDocumentRow.created_at, SessionDocumentRow.id) ) return tuple( SessionDocument( session_id=row.session_id, + guild_id=guild_id, target_id=row.target_id, provider=row.provider, document_id=row.document_id, url=row.url, created_at=row.created_at, ) - for row in rows + for row, guild_id in rows ) diff --git a/src/sturnus/infrastructure/documents/artefacts.py b/src/sturnus/infrastructure/documents/artefacts.py new file mode 100644 index 0000000..c5e896c --- /dev/null +++ b/src/sturnus/infrastructure/documents/artefacts.py @@ -0,0 +1,167 @@ +"""The stored protocol, sealed on the way in and opened on the way out. + +One class, and both processes hold it: the worker writes an artefact +through it (`sturnus.infrastructure.documents.sinks.DocumentObjectStore`) +and the API reads one back through it +(`sturnus.console.ports.DocumentArtefacts`). Two classes would be two +places the envelope's associated data is spelled, and an artefact sealed +under a context the reader does not reproduce is an artefact nobody can +open -- a failure that surfaces months later, on somebody's link to last +quarter's minutes, and not at the moment the mistake was made. + +**Why this exists at all.** A Markdown export is every word every +participant said, in one object. Every other object in that bucket -- the +recordings, and the stored spectrograms beside them -- is ciphertext; this +was the only one that was not. Access control in front of it +(`sturnus.console.routes_documents`) is a rule this process enforces; +encryption is a property of the bytes, and the two answer different +questions about a bucket somebody has a copy of. + +**The key an artefact is sealed under, and why it is not the recording's.** +A recording is sealed under a per-session data key wrapped into +`transcription_job`, and the retention sweep ends that recording's life +after `audio_retention_days`. An export artefact is not audio: it belongs +to the protocol, which deliberately outlives the recording -- a transcript +answers `200` with `audio_available: false` rather than `404` precisely +because the retention window governs the recording and not the record of +the meeting. Sealing an artefact under a session data key would therefore +tie a document meant to last to a key whose whole purpose is to stop +lasting, and the failure would be silent: every stored Markdown and HTML +export unreadable on the day the window closes, noticed by whoever next +opened an old document. + +Two further facts make the session key wrong here even setting the +lifetime aside. A data key is per *job*, which is per *speaker*; a +session's protocol merges every speaker, so there is no one session key +to choose and picking a participant's would bind a whole meeting's +document to one person's row. And the retention sweep deletes objects, +which means the correct hardening of that sweep -- clearing +`wrapped_data_key` when the audio goes -- would take the protocols with +it. + +So an artefact carries a data key of its own, generated per write and +wrapped under the master key with `secret_context(PURPOSE, guild_id)`: +bound to the guild and to the purpose, so an object relocated onto +another guild's key fails to authenticate rather than handing that guild +somebody else's meeting; and bound to nothing whose deletion is somebody's +scheduled job. `tests/application/test_retention.py` pins the lifetime, +so that re-binding these objects to the audio's fails a test rather than +a link. +""" + +from __future__ import annotations + +import logging +from typing import Protocol + +from cryptography.exceptions import InvalidTag + +from sturnus.domain.errors import UnreadableArtefact +from sturnus.infrastructure.crypto import ( + KeyWrapper, + is_sealed_artefact, + open_artefact, + seal_artefact, + secret_context, +) +from sturnus.observability.events import Event, log_event + +log = logging.getLogger(__name__) + +__all__ = ["PURPOSE", "DocumentBytes", "SealedArtefacts"] + +#: What this artefact's wrapped key is bound to, beside the guild. A +#: literal from this source, and distinct from `export-target` -- one +#: guild holds both a destination's credential and that destination's +#: artefacts, and binding to the guild alone would leave the two +#: interchangeable within it. See +#: `sturnus.infrastructure.crypto.secret_context`. +PURPOSE = "export-artefact" + +#: What the object is stored as. Not the format's own media type: these +#: bytes are an envelope, not a document, and a bucket listing that calls +#: them `text/markdown` invites the next reader to treat them as text. The +#: real media type is the format registry's answer and reaches the browser +#: from there (`sturnus.application.export_formats.ExportFormat`). +_SEALED_MEDIA_TYPE = "application/octet-stream" + + +class DocumentBytes(Protocol): + """Whole small objects in the bucket. `S3DocumentStore`, structurally. + + Structural rather than the concrete class, for the reason every other + collaborator in this codebase is: what this module is *about* is the + envelope, and a test of the envelope should not need a bucket to + exercise it. `KeyError` for an object that is not there. + """ + + async def put(self, key: str, body: bytes, content_type: str) -> None: ... + + async def get(self, key: str) -> bytes: ... + + +class SealedArtefacts: + """Seals a rendered protocol into the object store, and opens it back. + + Holds the process's `KeyWrapper` -- the same one the export targets' + credentials are wrapped with, handed in rather than built from the + master key again, so a rotation is threaded through one decode per + process rather than one per collaborator. + """ + + def __init__(self, store: DocumentBytes, keys: KeyWrapper) -> None: + self._store = store + self._keys = keys + + async def put_sealed(self, key: str, body: bytes, *, guild_id: int) -> None: + """Seals `body` under a data key of its own and writes the object. + + There is no unsealed spelling of this method, on this class or on + the port it satisfies: a protocol written in clear is the thing + this module exists to make unavailable rather than discouraged. + + Nothing is logged here. `ObjectStoreSink.create` already writes + one line per stored artefact, with the session and the target on + it; a second line for the same act, from the layer underneath, + would be the same event told twice with less context. + """ + sealed = seal_artefact(body, self._keys, secret_context(PURPOSE, guild_id)) + await self._store.put(key, sealed, _SEALED_MEDIA_TYPE) + + async def get(self, key: str, *, guild_id: int) -> bytes: + """The protocol's bytes, or a refusal. + + `KeyError` when the object is not there, which is + `S3DocumentStore.get`'s own answer and an ordinary one -- a row + can outlive its object. `UnreadableArtefact` when the object is + there and does not open, which is not ordinary, and which the + caller reports as the finding it is rather than as a tidy-up. + + **An object stored before this format existed is served as it + is.** Those artefacts are plaintext Markdown and HTML written by + the release that introduced export targets, nothing sweeps them, + and refusing them would turn every link already handed to a + participant into a 404 in the name of protecting a document that + link is the only way to reach. They are logged at WARNING on + every read, so the corpus is visible while it drains: a re-export + of the session overwrites the object at the same address, sealed. + """ + stored = await self._store.get(key) + if not is_sealed_artefact(stored): + log_event( + log, + logging.WARNING, + Event.SESSION_EXPORT_UNSEALED, + "Served a protocol stored before export artefacts were sealed", + guild_id=guild_id, + bytes=len(stored), + ) + return stored + try: + return open_artefact(stored, self._keys, secret_context(PURPOSE, guild_id)) + except (InvalidTag, ValueError) as exc: + # Raised rather than logged here. The line worth writing has + # the session and the target on it, and this class knows + # neither; `sturnus.console.routes_documents.read_document` + # does, and logs it there beside the refusal it turns into. + raise UnreadableArtefact("a stored protocol did not open") from exc diff --git a/src/sturnus/infrastructure/documents/sinks.py b/src/sturnus/infrastructure/documents/sinks.py index c23a42b..ba63d92 100644 --- a/src/sturnus/infrastructure/documents/sinks.py +++ b/src/sturnus/infrastructure/documents/sinks.py @@ -32,9 +32,22 @@ class DocumentObjectStore(Protocol): - """Where a rendered protocol is written. `S3DocumentStore`, structurally.""" + """Where a rendered protocol is written. `SealedArtefacts`, structurally. + + **One method, and its name is the decision.** A Markdown export is + every word every participant said, in one object, in the bucket that + otherwise holds nothing but ciphertext; a port offering a plain `put` + beside this one would be a port somebody eventually writes a protocol + through in clear. There is no such method, so there is nothing to + choose between. + + `guild_id` is here because sealing binds the artefact's key to the + guild and the purpose -- see + `sturnus.infrastructure.documents.artefacts.SealedArtefacts` and + `sturnus.infrastructure.crypto.seal_artefact`. + """ - async def put(self, key: str, body: bytes, content_type: str) -> None: ... + async def put_sealed(self, key: str, body: bytes, *, guild_id: int) -> None: ... def document_path(session_id: int, target_id: int) -> str: @@ -70,14 +83,14 @@ def __init__( console_origin: str, session_id: int, target_id: int, - media_type: str, + guild_id: int, file_extension: str, ) -> None: self._store = store self._console_origin = console_origin.rstrip("/") self._session_id = session_id self._target_id = target_id - self._media_type = media_type + self._guild_id = guild_id self._file_extension = file_extension async def create(self, title: str, body: str, target: str) -> CreatedDocument: @@ -96,10 +109,19 @@ async def create(self, title: str, body: str, target: str) -> CreatedDocument: parameter stays because `DocumentSink` is one port and a sink that quietly took a different shape would not be interchangeable with the others. + + The artefact is **sealed**, and the port says so in its one + method's name. This object is the most sensitive thing in the + bucket -- a recording is one speaker, a protocol is every word + every participant said -- and until now it was the only thing in + it that was not ciphertext. What the media type described is + still true of the document and no longer true of the object, so + it is not written here at all: the console reads it back from the + format registry, which is where it always came from. """ key = document_key(target, self._session_id, self._target_id, self._file_extension) payload = body.encode("utf-8") - await self._store.put(key, payload, self._media_type) + await self._store.put_sealed(key, payload, guild_id=self._guild_id) # DEBUG and sizes only, matching `OutlineSink.create`: `title` is # derived from the transcript and `body` *is* the transcript. The # key is not logged either -- `s3_key` is in `DENIED_NAMES`, and an @@ -161,7 +183,7 @@ def sink_for(self, destination: Destination) -> DocumentSink | None: console_origin=self._console_origin, session_id=destination.session_id, target_id=destination.target_id, - media_type=destination.format.media_type, + guild_id=destination.guild_id, file_extension=destination.format.file_extension, ) return None diff --git a/src/sturnus/infrastructure/objectstore.py b/src/sturnus/infrastructure/objectstore.py index bbda7b4..01e136f 100644 --- a/src/sturnus/infrastructure/objectstore.py +++ b/src/sturnus/infrastructure/objectstore.py @@ -160,17 +160,20 @@ class S3DocumentStore: what `S3AudioStore` refuses to offer -- a protocol is tens of kilobytes of text and a recording is hundreds of megabytes of somebody's voice. - **The stored object is not encrypted, and that is a consequence of a - scope decision rather than an oversight.** Envelope encryption needs - somewhere to put a wrapped data key, `session_document` has no column - for one, and this change adds no migration. The content is the same - transcript `transcription_job.transcript` already holds in clear in the - database, so nothing is newly exposed to anybody who can read the - database; what *is* new is a bucket that until now held only ciphertext. - Access control is the console route in front of it, which is why the - URL a sink hands back points there and never at a presigned S3 URL -- - a presigned URL outlives the access rules that issued it, and the - participant rule this content sits behind is checked per request. + **This class moves bytes and does not decide what they are.** The + objects it carries today are sealed envelopes + (`sturnus.infrastructure.documents.artefacts.SealedArtefacts`, which + is what both processes actually hold): a protocol is every word every + participant said, and it was for one release the only thing in this + bucket that was not ciphertext. The sealing is deliberately *not* + here, because "put whole small object" and "seal a protocol" are + different jobs and a store that did both would be a store somebody + reaches for when they want the first. + + Access control is the console route in front of the artefact, which is + why the URL a sink hands back points there and never at a presigned S3 + URL -- a presigned URL outlives the access rules that issued it, and + the participant rule this content sits behind is checked per request. """ def __init__( diff --git a/src/sturnus/observability/events.py b/src/sturnus/observability/events.py index be79da7..c29f741 100644 --- a/src/sturnus/observability/events.py +++ b/src/sturnus/observability/events.py @@ -160,6 +160,14 @@ class Event(StrEnum): #: `pdf` yet it is a configured intention, and it repeats on every #: publish for as long as the row exists. SESSION_EXPORT_SKIPPED = "session.export_skipped" + #: A stored protocol was served that had been written in plaintext, + #: before export artefacts were sealed. WARNING rather than INFO: it + #: is the only signal that unencrypted meetings are still in the + #: bucket, and the count of these lines going to zero is how an + #: operator knows that corpus has drained. Not an error -- refusing + #: the object would turn a link somebody already holds into a 404 + #: over bytes that are already written. + SESSION_EXPORT_UNSEALED = "session.export_unsealed" RETENTION_SWEPT = "retention.swept" RETENTION_FAILED = "retention.failed" #: A guild that asked for `spectrograms_by_default` got one: the diff --git a/tests/application/test_exporting.py b/tests/application/test_exporting.py index 5a7e3d1..b4df3d1 100644 --- a/tests/application/test_exporting.py +++ b/tests/application/test_exporting.py @@ -29,6 +29,7 @@ T0 = datetime(2026, 8, 19, 20, 0, 0, tzinfo=UTC) NOW = T0 + timedelta(hours=2) SESSION = 77 +GUILD = 1 TEMPLATE = ( Path(__file__).parent.parent.parent @@ -62,7 +63,7 @@ def target( ) -> ExportTarget: return ExportTarget( id=target_id, - guild_id=1, + guild_id=GUILD, format=format, name=name, target=where, @@ -80,7 +81,12 @@ def destination( entry = format_named(format) assert entry is not None return exporting.Destination( - session_id=SESSION, target_id=target_id, format=entry, target=where, provider=format + session_id=SESSION, + target_id=target_id, + format=entry, + target=where, + provider=format, + guild_id=GUILD, ) @@ -441,6 +447,7 @@ async def test_a_failure_beside_a_destination_already_published_does_not_raise() def recorded(target_id: int, url: str = "https://outline/1") -> SessionDocument: return SessionDocument( session_id=SESSION, + guild_id=GUILD, target_id=target_id, provider=OUTLINE, document_id="doc-1", diff --git a/tests/application/test_retention.py b/tests/application/test_retention.py index f837dfa..3adecb7 100644 --- a/tests/application/test_retention.py +++ b/tests/application/test_retention.py @@ -1,6 +1,8 @@ from datetime import UTC, datetime, timedelta from sturnus.application.retention import expired_jobs, sweep_expired_audio +from sturnus.infrastructure.crypto import KeyWrapper +from sturnus.infrastructure.documents.artefacts import SealedArtefacts T0 = datetime(2026, 8, 19, 20, 0, 0, tzinfo=UTC) @@ -166,3 +168,79 @@ async def test_a_spectrogram_that_would_not_delete_leaves_the_job_unstamped() -> assert store.deleted == ["a"] assert jobs.deleted == [] + + +# --------------------------------------------------------------------------- +# The rule a stored *protocol* exists under, which is the opposite one +# +# A protocol is not audio. `audio_retention_days` governs the recording, +# and the record of the meeting deliberately outlives it: a transcript +# answers `200` with `audio_available: false` rather than `404` for exactly +# that reason. So this sweep must not touch a session's stored protocol, +# and -- the half that is invisible from the sweep -- a stored protocol +# must not be sealed under anything the sweep destroys. +# +# Sealing an export artefact under the session data key would satisfy every +# other test in this file and make every stored Markdown and HTML export +# unreadable on day thirty-one, silently, noticed by whoever next opened an +# old document. These two tests are what that mistake has to get past. +# --------------------------------------------------------------------------- + +MASTER = b"0" * 32 +GUILD = 4711 +PROTOCOL_KEY = "protocols/1/7.md" + + +class FakeObjects: + """The bucket, as a dictionary, for the two things stored in it.""" + + def __init__(self) -> None: + self.objects: dict[str, bytes] = {} + + async def put(self, key: str, body: bytes, _content_type: str) -> None: + self.objects[key] = body + + async def get(self, key: str) -> bytes: + return self.objects[key] + + +async def test_the_sweep_does_not_delete_a_sessions_stored_protocol() -> None: + """The sweep deletes what the job names -- its audio and its picture -- + and nothing else in the bucket. + + Adding the protocol to this list would be deleting the record of a + meeting because the recording of it expired, which is not what + `audio_retention_days` is.""" + jobs = FakeJobs([job(1, T0 - timedelta(seconds=1), s3_key="a", spectrogram_key="a.spec")]) + store = FakeStore() + + await sweep_expired_audio(jobs, store, T0) + + assert PROTOCOL_KEY not in store.deleted + assert store.deleted == ["a", "a.spec"] + + +async def test_a_stored_protocol_still_opens_after_its_recording_is_swept() -> None: + """The failure this pins is a data-loss bug wearing the costume of a + security fix. + + An export artefact is sealed under a data key of its own, wrapped under + the master key and bound to the guild and the purpose -- never under + the session data key the sweep's job row carries. Everything that key + depends on is destroyed here first: the audio object, the picture, and + the row's own key material. The protocol still opens, with the master + key and the guild and nothing else.""" + artefacts = SealedArtefacts(FakeObjects(), KeyWrapper(master_key=MASTER, key_id="k1")) + await artefacts.put_sealed(PROTOCOL_KEY, b"# Minutes\n", guild_id=GUILD) + + expired = job(1, T0 - timedelta(seconds=1), s3_key="a", spectrogram_key="a.spec") + jobs = FakeJobs([expired]) + await sweep_expired_audio(jobs, FakeStore(), T0) + # What the sweep leaves behind today, plus the obvious hardening of it + # that a later change would make: the row keeps no key material for a + # recording that no longer exists. Either way the protocol is not + # holding on to it. + expired["wrapped_data_key"] = None + expired["encryption_key_id"] = None + + assert await artefacts.get(PROTOCOL_KEY, guild_id=GUILD) == b"# Minutes\n" diff --git a/tests/console/conftest.py b/tests/console/conftest.py index f6be8ec..6ba771f 100644 --- a/tests/console/conftest.py +++ b/tests/console/conftest.py @@ -85,6 +85,7 @@ TagUse, ) from sturnus.domain import preferences +from sturnus.domain.errors import UnreadableArtefact from sturnus.domain.exports import ExportTarget, SessionDocument from sturnus.domain.oauth_clients import GuildOAuthClient, SlugUnavailable, is_valid_slug from sturnus.domain.onboarding import SetupIntent @@ -1250,12 +1251,36 @@ async def document_of(self, session_id: int, target_id: int) -> SessionDocument class FakeArtefacts: - """The object store, as a dictionary.""" + """The object store, as a dictionary, with both of the seal's answers. + + `unreadable` names keys that are in the store and do not open -- a + wrong master key, an envelope sealed under another guild, a body + edited in the bucket. The real adapter + (`sturnus.infrastructure.documents.artefacts.SealedArtefacts`) has + exactly these two failures and they are not the same failure: one is a + row outliving its object, the other is somebody's meeting failing to + authenticate. + + `asked` records the `(key, guild_id)` of every read, because which + guild the route supplies is what the artefact's key is bound to -- a + route passing the wrong one would produce a 404 that looks exactly + like a missing object. + """ - def __init__(self, objects: dict[str, bytes] | None = None) -> None: + def __init__( + self, + objects: dict[str, bytes] | None = None, + *, + unreadable: frozenset[str] = frozenset(), + ) -> None: self.objects = objects or {} + self.unreadable = unreadable + self.asked: list[tuple[str, int]] = [] - async def get(self, key: str) -> bytes: + async def get(self, key: str, *, guild_id: int) -> bytes: + self.asked.append((key, guild_id)) + if key in self.unreadable: + raise UnreadableArtefact("a stored protocol did not open") return self.objects[key] diff --git a/tests/console/test_document_routes.py b/tests/console/test_document_routes.py index f04cdd1..f2df013 100644 --- a/tests/console/test_document_routes.py +++ b/tests/console/test_document_routes.py @@ -34,6 +34,7 @@ SESSION_COOKIE = "sturnus_session" SESSION = 42 +GUILD = 4711 T0 = datetime(2026, 8, 21, 12, 0, 0, tzinfo=UTC) MARKDOWN_KEY = "protocols/42/7.md" @@ -45,6 +46,7 @@ def document( ) -> SessionDocument: return SessionDocument( session_id=SESSION, + guild_id=GUILD, target_id=target_id, provider=provider, document_id=document_id, @@ -294,3 +296,55 @@ async def test_neither_route_answers_without_a_session( ) -> None: client = await aiohttp_client(api([document(7, "markdown", MARKDOWN_KEY)])) assert (await client.get(path)).status == 401 + + +# --------------------------------------------------------------------------- +# The seal, from this side of it +# --------------------------------------------------------------------------- + + +async def test_the_artefact_is_asked_for_under_the_guild_the_row_names( + aiohttp_client: AiohttpClientFactory, cookies: dict[str, str] +) -> None: + """A stored protocol is sealed under a key bound to its guild, and the + binding has to come from the row rather than from the object -- an + envelope carrying the guild it was filed under would authenticate just + as happily after being moved onto another guild's key. + + So the guild the route supplies is load-bearing, and supplying the + wrong one fails in a way indistinguishable from a missing object. This + is the only place that can be checked.""" + artefacts = FakeArtefacts({MARKDOWN_KEY: b"# Minutes\n"}) + client = await aiohttp_client( + build_test_api( + reads=FakeReads(sessions=(attended(),)), + documents=FakeSessionDocuments({SESSION: [document(7, "markdown", MARKDOWN_KEY)]}), + artefacts=artefacts, + ) + ) + response = await client.get(f"/api/sessions/{SESSION}/documents/7", cookies=cookies) + assert response.status == 200 + assert artefacts.asked == [(MARKDOWN_KEY, GUILD)] + + +async def test_an_artefact_that_does_not_open_is_a_404_and_not_a_500( + aiohttp_client: AiohttpClientFactory, cookies: dict[str, str] +) -> None: + """The object is there and does not open: a wrong master key, an + envelope sealed under a guild this row does not name, a body edited in + the bucket. There is nothing here to serve, so the reader gets the + same refusal every other reason gets -- and the log gets a different + `reason`, because "the sweep took it" and "it failed to authenticate" + are different mornings for whoever is on call.""" + client = await aiohttp_client( + build_test_api( + reads=FakeReads(sessions=(attended(),)), + documents=FakeSessionDocuments({SESSION: [document(7, "markdown", MARKDOWN_KEY)]}), + artefacts=FakeArtefacts( + {MARKDOWN_KEY: b"# Minutes\n"}, unreadable=frozenset({MARKDOWN_KEY}) + ), + ) + ) + response = await client.get(f"/api/sessions/{SESSION}/documents/7", cookies=cookies) + assert response.status == 404 + assert "Minutes" not in await response.text() diff --git a/tests/infrastructure/test_crypto.py b/tests/infrastructure/test_crypto.py index a1bf97d..90d9f6e 100644 --- a/tests/infrastructure/test_crypto.py +++ b/tests/infrastructure/test_crypto.py @@ -5,10 +5,15 @@ from cryptography.exceptions import InvalidTag from sturnus.infrastructure.crypto import ( + ARTEFACT_MAGIC, CHUNK_SIZE, + MAGIC, KeyWrapper, decrypt_file, encrypt_file, + is_sealed_artefact, + open_artefact, + seal_artefact, secret_context, ) @@ -170,3 +175,111 @@ def test_a_data_key_can_be_bound_too() -> None: w = wrapper() key = w.new_data_key(secret_context("export", 1)) assert w.unwrap(key.wrapped, secret_context("export", 1)) == key.plaintext + + +# --------------------------------------------------------------------------- +# The sealed artefact envelope +# --------------------------------------------------------------------------- +# +# A rendered protocol, sealed in one piece and carrying the wrapped key +# that opens it. The properties worth pinning are the two that make this a +# different shape from `encrypt_file`: the object is self-contained, and +# the key inside it is bound to a context the reader has to supply from +# somewhere other than the object. + + +def test_a_sealed_artefact_round_trips() -> None: + w = wrapper() + aad = secret_context("export-artefact", 7) + sealed = seal_artefact(b"# Minutes\n", w, aad) + assert open_artefact(sealed, w, aad) == b"# Minutes\n" + + +def test_a_sealed_artefact_does_not_contain_its_plaintext() -> None: + w = wrapper() + sealed = seal_artefact(b"Anna said something", w, secret_context("export-artefact", 7)) + assert b"Anna said something" not in sealed + + +def test_a_sealed_artefact_names_itself() -> None: + """The envelope is self-describing so a reader can tell it apart from + the plaintext artefacts written before it existed, and from the chunked + recording format, without being told which it is holding.""" + sealed = seal_artefact(b"body", wrapper(), secret_context("export-artefact", 7)) + assert sealed.startswith(ARTEFACT_MAGIC) + assert is_sealed_artefact(sealed) + assert not is_sealed_artefact(b"# Minutes\n") + assert not is_sealed_artefact(b"") + + +def test_two_seals_of_one_body_differ() -> None: + """A fresh data key per artefact, so two objects never share a key and + a nonce is never reused under one.""" + w = wrapper() + aad = secret_context("export-artefact", 7) + assert seal_artefact(b"body", w, aad) != seal_artefact(b"body", w, aad) + + +def test_an_artefact_sealed_for_one_guild_does_not_open_for_another() -> None: + """The whole point of binding the key to a context the object does not + carry: an object copied onto another guild's key fails to authenticate + instead of handing that guild somebody else's meeting.""" + w = wrapper() + sealed = seal_artefact(b"body", w, secret_context("export-artefact", 7)) + with pytest.raises(InvalidTag): + open_artefact(sealed, w, secret_context("export-artefact", 8)) + + +def test_an_artefact_does_not_open_under_another_purpose() -> None: + w = wrapper() + sealed = seal_artefact(b"body", w, secret_context("export-artefact", 7)) + with pytest.raises(InvalidTag): + open_artefact(sealed, w, secret_context("export-target", 7)) + + +def test_an_artefact_does_not_open_under_another_master_key() -> None: + sealed = seal_artefact(b"body", wrapper(), secret_context("export-artefact", 7)) + other = KeyWrapper(master_key=b"1" * 32, key_id="k1") + with pytest.raises(InvalidTag): + open_artefact(sealed, other, secret_context("export-artefact", 7)) + + +def test_a_tampered_body_does_not_open() -> None: + w = wrapper() + aad = secret_context("export-artefact", 7) + sealed = bytearray(seal_artefact(b"body", w, aad)) + sealed[-1] ^= 0xFF + with pytest.raises(InvalidTag): + open_artefact(bytes(sealed), w, aad) + + +def test_something_that_is_not_an_artefact_is_refused_before_any_key_is_touched() -> None: + w = wrapper() + with pytest.raises(ValueError): + open_artefact(b"# Minutes\n", w, secret_context("export-artefact", 7)) + + +def test_a_truncated_artefact_is_refused() -> None: + w = wrapper() + aad = secret_context("export-artefact", 7) + sealed = seal_artefact(b"body", w, aad) + with pytest.raises(ValueError): + open_artefact(sealed[: len(ARTEFACT_MAGIC) + 1], w, aad) + + +def test_an_artefact_claiming_a_longer_key_than_it_holds_is_refused() -> None: + """A length prefix read out of an object is a length an attacker + chooses, so it is checked against what is actually there.""" + w = wrapper() + aad = secret_context("export-artefact", 7) + sealed = bytearray(seal_artefact(b"body", w, aad)) + sealed[len(ARTEFACT_MAGIC) : len(ARTEFACT_MAGIC) + 2] = b"\xff\xff" + with pytest.raises(ValueError): + open_artefact(bytes(sealed), w, aad) + + +def test_an_artefact_is_not_the_chunked_recording_format() -> None: + """Two formats, one family, and neither reader accepts the other's + bytes: `decrypt_file` refuses this on its magic.""" + sealed = seal_artefact(b"body", wrapper(), secret_context("export-artefact", 7)) + assert not sealed.startswith(MAGIC) diff --git a/tests/infrastructure/test_document_sinks.py b/tests/infrastructure/test_document_sinks.py index 0410db8..951981a 100644 --- a/tests/infrastructure/test_document_sinks.py +++ b/tests/infrastructure/test_document_sinks.py @@ -1,10 +1,18 @@ """The object-store sink, and the resolver that decides which sink runs. -The load-bearing assertion in this file is the one about the URL. A -presigned S3 URL would satisfy `CreatedDocument` and would be wrong: it -works for anybody it is forwarded to, it keeps working after a participation -ends, and nothing can revoke it. The URL a protocol's link carries has to -point back at the console, where the rule is checked on every request. +Two load-bearing assertions in this file. The first is the one about the +URL: a presigned S3 URL would satisfy `CreatedDocument` and would be +wrong -- it works for anybody it is forwarded to, it keeps working after a +participation ends, and nothing can revoke it. The URL a protocol's link +carries has to point back at the console, where the rule is checked on +every request. + +The second is that **what lands in the bucket is not the protocol**. A +Markdown export is every word every participant said; access control is +the console route in front of it, encryption is a property of the object, +and the two answer different questions about a bucket somebody has a copy +of. The sink is given a store whose only write method seals, so there is +no plaintext spelling of this act left to test for. """ from __future__ import annotations @@ -19,18 +27,37 @@ from sturnus.application.documents import CreatedDocument, DocumentSink from sturnus.application.export_formats import HTML, MARKDOWN, OUTLINE, format_named from sturnus.application.exporting import Destination +from sturnus.domain.errors import UnreadableArtefact +from sturnus.infrastructure.crypto import KeyWrapper, is_sealed_artefact +from sturnus.infrastructure.documents.artefacts import SealedArtefacts from sturnus.infrastructure.documents.sinks import DocumentSinks, ObjectStoreSink from sturnus.infrastructure.objectstore import S3DocumentStore BUCKET = "sturnus-audio" T0 = datetime(2026, 8, 19, 20, 0, 0, tzinfo=UTC) +GUILD = 4711 +MASTER = b"0" * 32 + + +def sealed_store() -> SealedArtefacts: + return SealedArtefacts( + S3DocumentStore(endpoint=None, bucket=BUCKET, access_key="ak", secret_key="sk"), + KeyWrapper(master_key=MASTER, key_id="k1"), + ) @pytest.fixture -def store() -> Iterator[S3DocumentStore]: +def store() -> Iterator[SealedArtefacts]: with mock_aws(): boto3.client("s3", region_name="us-east-1").create_bucket(Bucket=BUCKET) - yield S3DocumentStore(endpoint=None, bucket=BUCKET, access_key="ak", secret_key="sk") + yield sealed_store() + + +def raw(key: str) -> bytes: + """What is actually in the bucket, read without going through the seal.""" + stored = boto3.client("s3", region_name="us-east-1").get_object(Bucket=BUCKET, Key=key) + body: bytes = stored["Body"].read() + return body def destination( @@ -39,11 +66,30 @@ def destination( entry = format_named(format) assert entry is not None return Destination( - session_id=42, target_id=target_id, format=entry, target=where, provider=format + session_id=42, + target_id=target_id, + format=entry, + target=where, + provider=format, + guild_id=GUILD, ) -def sink(store: S3DocumentStore, format: str = MARKDOWN) -> ObjectStoreSink: +def at(store: SealedArtefacts, target_id: int) -> ObjectStoreSink: + """The Markdown sink for one of a guild's several destinations.""" + entry = format_named(MARKDOWN) + assert entry is not None + return ObjectStoreSink( + store, + console_origin="https://sturnus.example", + session_id=42, + target_id=target_id, + guild_id=GUILD, + file_extension=entry.file_extension, + ) + + +def sink(store: SealedArtefacts, format: str = MARKDOWN) -> ObjectStoreSink: entry = format_named(format) assert entry is not None return ObjectStoreSink( @@ -51,7 +97,7 @@ def sink(store: S3DocumentStore, format: str = MARKDOWN) -> ObjectStoreSink: console_origin="https://sturnus.example", session_id=42, target_id=7, - media_type=entry.media_type, + guild_id=GUILD, file_extension=entry.file_extension, ) @@ -61,25 +107,36 @@ def sink(store: S3DocumentStore, format: str = MARKDOWN) -> ObjectStoreSink: # --------------------------------------------------------------------------- -async def test_the_rendered_protocol_reaches_the_object_store(store: S3DocumentStore) -> None: +async def test_the_rendered_protocol_reaches_the_object_store(store: SealedArtefacts) -> None: await sink(store).create("A meeting", "# Minutes\n", "protocols") - stored = boto3.client("s3", region_name="us-east-1").get_object( - Bucket=BUCKET, Key="protocols/42/7.md" - ) - assert stored["Body"].read() == b"# Minutes\n" - - -async def test_the_object_is_stored_under_its_own_media_type(store: S3DocumentStore) -> None: - """A browser handed `text/html` renders the protocol; handed - `binary/octet-stream` it offers to download it.""" + assert await store.get("protocols/42/7.md", guild_id=GUILD) == b"# Minutes\n" + + +async def test_what_lands_in_the_bucket_is_not_the_protocol(store: SealedArtefacts) -> None: + """The gap this closes. Every other object in this bucket -- the + recordings, the stored spectrograms -- is ciphertext, and a Markdown + export is the most sensitive of the lot: every word every participant + said, in one object.""" + await sink(store).create("A meeting", "Anna said something\n", "protocols") + stored = raw("protocols/42/7.md") + assert b"Anna said something" not in stored + assert is_sealed_artefact(stored) + + +async def test_the_object_does_not_claim_to_be_a_document(store: SealedArtefacts) -> None: + """The media type describes the protocol and no longer describes the + object. A bucket listing that calls a sealed envelope `text/html` + invites the next reader to treat it as text; the real media type + reaches the browser from the format registry, which is where it always + came from.""" await sink(store, HTML).create("A meeting", "

hi

", "protocols") stored = boto3.client("s3", region_name="us-east-1").get_object( Bucket=BUCKET, Key="protocols/42/7.html" ) - assert stored["ContentType"] == "text/html; charset=utf-8" + assert stored["ContentType"] == "application/octet-stream" -async def test_the_document_id_is_the_object_key(store: S3DocumentStore) -> None: +async def test_the_document_id_is_the_object_key(store: SealedArtefacts) -> None: """`session_document.document_id` is what the console route reads back to find the bytes, so it has to be a real identifier in the store that holds them rather than a second, invented one.""" @@ -88,7 +145,7 @@ async def test_the_document_id_is_the_object_key(store: S3DocumentStore) -> None async def test_the_url_points_at_the_console_and_not_at_the_object_store( - store: S3DocumentStore, + store: SealedArtefacts, ) -> None: """The whole argument of §3.2. A presigned S3 URL outlives the access rules that issued it and cannot be revoked; this one is answered by a @@ -99,40 +156,100 @@ async def test_the_url_points_at_the_console_and_not_at_the_object_store( assert BUCKET not in created.url -async def test_two_destinations_of_one_format_get_two_objects(store: S3DocumentStore) -> None: +async def test_two_destinations_of_one_format_get_two_objects(store: SealedArtefacts) -> None: """A guild publishing Markdown to two prefixes wants two artefacts. One key for both would have the second overwrite the first.""" - entry = format_named(MARKDOWN) - assert entry is not None for target_id, prefix in ((7, "team"), (9, "archive")): - await ObjectStoreSink( - store, - console_origin="https://sturnus.example", - session_id=42, - target_id=target_id, - media_type=entry.media_type, - file_extension=entry.file_extension, - ).create("A meeting", f"body {target_id}", prefix) - assert await store.get("team/42/7.md") == b"body 7" - assert await store.get("archive/42/9.md") == b"body 9" + await at(store, target_id).create("A meeting", f"body {target_id}", prefix) + assert await store.get("team/42/7.md", guild_id=GUILD) == b"body 7" + assert await store.get("archive/42/9.md", guild_id=GUILD) == b"body 9" + + +async def test_two_artefacts_never_share_a_key(store: SealedArtefacts) -> None: + """A data key per artefact, so no two objects are sealed under one and + no nonce is ever reused. Two identical bodies at two addresses are two + different envelopes.""" + for target_id, prefix in ((7, "team"), (9, "archive")): + await at(store, target_id).create("A meeting", "the same body", prefix) + assert raw("team/42/7.md") != raw("archive/42/9.md") async def test_a_re_export_replaces_the_artefact_at_the_same_address( - store: S3DocumentStore, + store: SealedArtefacts, ) -> None: """`SessionDocumentStore.record` upserts on `(session_id, target_id)`, so a second artefact at a second key would leave a row pointing at one - of two objects with nothing saying which is current.""" + of two objects with nothing saying which is current. + + It is also how a protocol written in clear before this format existed + becomes a sealed one: the object at that address is replaced, in + place.""" await sink(store).create("A meeting", "first", "protocols") created = await sink(store).create("A meeting", "second", "protocols") - assert await store.get(created.id) == b"second" + assert await store.get(created.id, guild_id=GUILD) == b"second" async def test_reading_an_artefact_that_is_not_there_is_a_key_error( - store: S3DocumentStore, + store: SealedArtefacts, ) -> None: with pytest.raises(KeyError): - await store.get("protocols/1/1.md") + await store.get("protocols/1/1.md", guild_id=GUILD) + + +# --------------------------------------------------------------------------- +# The seal, and what it refuses +# --------------------------------------------------------------------------- + + +async def test_an_artefact_does_not_open_for_another_guild(store: SealedArtefacts) -> None: + """What the associated data buys. Somebody who can write to the bucket + but cannot decrypt copies one guild's artefact onto another guild's + key; the second guild's reader gets a refusal rather than the first + guild's meeting.""" + created = await sink(store).create("A meeting", "# Minutes\n", "protocols") + with pytest.raises(UnreadableArtefact): + await store.get(created.id, guild_id=GUILD + 1) + + +async def test_an_artefact_does_not_open_under_another_master_key( + store: SealedArtefacts, +) -> None: + created = await sink(store).create("A meeting", "# Minutes\n", "protocols") + other = SealedArtefacts( + S3DocumentStore(endpoint=None, bucket=BUCKET, access_key="ak", secret_key="sk"), + KeyWrapper(master_key=b"1" * 32, key_id="k1"), + ) + with pytest.raises(UnreadableArtefact): + await other.get(created.id, guild_id=GUILD) + + +async def test_an_edited_object_does_not_open(store: SealedArtefacts) -> None: + """A protocol is served from the console's own origin and one of its + formats is HTML, so a body somebody can edit in the bucket is a page + somebody can edit into that origin. It is authenticated, not merely + encrypted.""" + created = await sink(store, HTML).create("A meeting", "

hi

", "protocols") + tampered = bytearray(raw(created.id)) + tampered[-1] ^= 0xFF + boto3.client("s3", region_name="us-east-1").put_object( + Bucket=BUCKET, Key=created.id, Body=bytes(tampered) + ) + with pytest.raises(UnreadableArtefact): + await store.get(created.id, guild_id=GUILD) + + +async def test_a_protocol_stored_before_the_seal_existed_is_still_served( + store: SealedArtefacts, +) -> None: + """The release that introduced export targets wrote these in clear. + Nothing sweeps them, and refusing them would turn every link already + handed to a participant into a 404 over bytes that are already in the + bucket. They are served, and logged on every read, until a re-export + replaces one.""" + boto3.client("s3", region_name="us-east-1").put_object( + Bucket=BUCKET, Key="protocols/42/7.md", Body=b"# Minutes\n" + ) + assert await store.get("protocols/42/7.md", guild_id=GUILD) == b"# Minutes\n" # --------------------------------------------------------------------------- @@ -152,14 +269,14 @@ def test_an_outline_destination_resolves_to_the_outline_sink() -> None: def test_an_object_store_destination_resolves_to_an_object_store_sink( - store: S3DocumentStore, + store: SealedArtefacts, ) -> None: sinks = DocumentSinks(objects=store, console_origin="https://sturnus.example") assert isinstance(sinks.sink_for(destination(MARKDOWN)), ObjectStoreSink) def test_the_resolver_branches_on_the_family_and_not_on_the_format( - store: S3DocumentStore, + store: SealedArtefacts, ) -> None: """`markdown` and `html` are two formats and one family, which is what makes `pdf` an entry in the registry rather than a change here.""" @@ -180,7 +297,7 @@ def test_a_deployment_with_no_outline_sink_serves_no_outline_destination() -> No async def test_the_console_origins_trailing_slash_does_not_double_up( - store: S3DocumentStore, + store: SealedArtefacts, ) -> None: """`STURNUS_CONSOLE_ORIGIN` is a Helm value somebody types, and a doubled slash in a link posted to Discord is a broken link."""