diff --git a/raven/rpc/transports/deliverables.py b/raven/rpc/transports/deliverables.py
index 576bce709..298d44524 100644
--- a/raven/rpc/transports/deliverables.py
+++ b/raven/rpc/transports/deliverables.py
@@ -118,6 +118,22 @@ async def download(request: web.Request) -> web.StreamResponse:
headers={
"Content-Type": record.media_type,
"Content-Disposition": _attachment(record.name),
+ # `register` reuses the token when a conversation delivers the
+ # same path again, so a rewritten file keeps its URL -- and the
+ # delivery card points an
straight at this route. Without
+ # a directive the browser invents a freshness lifetime from
+ # Last-Modified (about a tenth of the file's age) and answers
+ # from its own copy without asking, so an image the agent
+ # replaced went on rendering as the one it replaced.
+ # `no-cache` would also close that window: `web.FileResponse`
+ # derives its validator from the file now at this path, so a
+ # conditional request after a rewrite does get the new bytes.
+ # `no-store` is chosen for the two reasons that do separate
+ # them: it matches `/file` and `/knowledge/file`, which serve
+ # the same kind of file, and it is the only directive that
+ # keeps an agent's output out of the reader's disk cache. This
+ # route has no size ceiling, unlike those two.
+ "Cache-Control": "no-store",
},
)
@@ -134,6 +150,10 @@ async def download_archive(request: web.Request) -> web.StreamResponse:
headers={
"Content-Type": "application/zip",
"Content-Disposition": _attachment("deliverables.zip"),
+ # Same reason as the single-file route: a token names a
+ # delivery, not a fixed set of files, and the members behind it
+ # can be rewritten between two downloads of the same URL.
+ "Cache-Control": "no-store",
}
)
if request.method == "HEAD":
diff --git a/tests/test_rpc_transports_deliverables.py b/tests/test_rpc_transports_deliverables.py
index 63e9c25a1..a61e6a710 100644
--- a/tests/test_rpc_transports_deliverables.py
+++ b/tests/test_rpc_transports_deliverables.py
@@ -139,8 +139,9 @@ async def test_control_characters_in_a_filename_are_stripped(client) -> None:
async def test_archive_head_agrees_with_get(client) -> None:
- """The frontend pre-checks "Download all" with HEAD too, so the branch that
- skips the zip build must answer with the same status."""
+ """The route is registered with ``allow_head=True``, so a HEAD has to answer
+ what a GET would. The archive builds its zip only on the GET path, so HEAD
+ is a second path that could drift from it."""
one = _register(client.store, client.tmp_path, "a.txt", b"AAA")
ok = await client.head("/files/download-archive", params={"token": one.token})
@@ -205,6 +206,51 @@ async def test_an_unregistered_route_still_answers_404(client) -> None:
assert res.status == 404
+async def test_download_forbids_storing_so_a_rewritten_file_is_fetched_again(client) -> None:
+ """A deliverable that was rewritten in place must not come back from the
+ browser cache.
+
+ ``register`` reuses the token when a conversation delivers the same path
+ again, so a rewritten file keeps the URL it had -- and the delivery card
+ points an ``
`` straight at this route. With no ``Cache-Control`` the
+ browser invents a freshness lifetime from ``Last-Modified`` (about 10% of
+ the file's age) and answers from its own copy without asking. An image the
+ agent replaced then went on rendering as the one it replaced, which is
+ indistinguishable from a server that did not pick the change up.
+ """
+ record = _register(client.store, client.tmp_path)
+
+ res = await client.get("/files/download", params={"token": record.token})
+
+ assert res.headers["Cache-Control"] == "no-store"
+
+
+async def test_archive_download_forbids_storing_too(client) -> None:
+ """The same route for a set of files: a token whose member changed must not
+ be answered from a stored copy either."""
+ one = _register(client.store, client.tmp_path, "a.txt", b"AAA")
+
+ res = await client.get("/files/download-archive", params={"token": one.token})
+
+ assert res.headers["Cache-Control"] == "no-store"
+
+
+async def test_head_carries_the_directive_as_well(client) -> None:
+ """The directive has to be on the response HEAD is answered with.
+
+ HEAD never reaches the archive's zip build: it returns through the early
+ exit before that work starts, so the directive has to sit on the response
+ the constructor builds rather than be set on the GET path afterwards."""
+ record = _register(client.store, client.tmp_path)
+ one = _register(client.store, client.tmp_path, "a.txt", b"AAA")
+
+ file_head = await client.head("/files/download", params={"token": record.token})
+ archive_head = await client.head("/files/download-archive", params={"token": one.token})
+
+ assert file_head.headers["Cache-Control"] == "no-store"
+ assert archive_head.headers["Cache-Control"] == "no-store"
+
+
async def test_resolve_download_drops_stale_entry(tmp_path) -> None:
store = DeliverableStore(tmp_path / "deliverables.json")
record = _register(store, tmp_path)