From a505eae2f675347f9eca7a1da5fe5ee13a80af52 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Wed, 26 Aug 2026 14:50:14 +0100 Subject: [PATCH 1/3] Answer a password-protected document as unreadable with cause protected Applications rendering a lock placeholder for protected PDFs matched mutool's stderr phrase in the failure message, which nothing specified. A ProtectedDocument refusal now carries cause: protected onto the wire: an UnreadableInput subclass may declare the failure's cause, run! maps stderr patterns onto the subclass to raise, and the worker reports the declared cause. --- .../hot_cell/server/previewers/pdf/mutool.rb | 3 ++- .../hot_cell/server/tool_operation.rb | 15 ++++++++++++--- .../test/fixtures/protected.pdf | Bin 0 -> 2872 bytes .../test/previewers_test.rb | 14 ++++++++++++++ hotcell-core/lib/hot_cell/codes.rb | 3 +++ hotcell-server/lib/hot_cell/server/errors.rb | 9 +++++++-- hotcell-server/lib/hot_cell/worker.rb | 3 ++- 7 files changed, 40 insertions(+), 7 deletions(-) create mode 100644 activestorage-hotcell-server/test/fixtures/protected.pdf diff --git a/activestorage-hotcell-server/lib/active_storage/hot_cell/server/previewers/pdf/mutool.rb b/activestorage-hotcell-server/lib/active_storage/hot_cell/server/previewers/pdf/mutool.rb index 5f72a47..c2d8bde 100644 --- a/activestorage-hotcell-server/lib/active_storage/hot_cell/server/previewers/pdf/mutool.rb +++ b/activestorage-hotcell-server/lib/active_storage/hot_cell/server/previewers/pdf/mutool.rb @@ -22,7 +22,8 @@ class Mutool < Pdf # separate change. def render(source, destination, page:, resolution:) run! "mutool", "draw", "-F", "png", "-r", resolution.to_s, "-o", destination.path, - source.fd_path, page.to_s, pass: [ source.to_io ] + source.fd_path, page.to_s, pass: [ source.to_io ], + causes: { /cannot authenticate password/ => ProtectedDocument } end def tool diff --git a/activestorage-hotcell-server/lib/active_storage/hot_cell/server/tool_operation.rb b/activestorage-hotcell-server/lib/active_storage/hot_cell/server/tool_operation.rb index 986679b..08cabfa 100644 --- a/activestorage-hotcell-server/lib/active_storage/hot_cell/server/tool_operation.rb +++ b/activestorage-hotcell-server/lib/active_storage/hot_cell/server/tool_operation.rb @@ -20,16 +20,25 @@ class ToolOperation < Operation # UnreadableInput is always in the rescue list, and this descends from it. class UnreadableDocument < ::HotCell::UnreadableInput; end + # The document demands credentials the cell does not have. + class ProtectedDocument < UnreadableDocument + def self.cause + ::HotCell::Codes::PROTECTED + end + end + private # Runs the tool with unsetenv_others and a written environment, and turns a non-zero exit into # `unreadable`. The tool's own stderr is the only useful diagnostic, and it is attacker-influenced, # so it is capped and scrubbed on its way onto the wire like any other error message. - def run!(*command, pass: []) + # `causes` maps stderr patterns to the UnreadableDocument subclass to raise instead. + def run!(*command, pass: [], causes: {}) result = run_tool(*command, pass: pass) return result if result.ok? - raise UnreadableDocument, "#{command.first} exited #{result.status.exitstatus}: " \ - "#{result.err.to_s.strip[0, 200]}" + error = causes.detect { |pattern, _| result.err.to_s.match?(pattern) }&.last || UnreadableDocument + raise error, "#{command.first} exited #{result.status.exitstatus}: " \ + "#{result.err.to_s.strip[0, 200]}" end # An empty output means the tool said it succeeded and produced nothing, which the client would diff --git a/activestorage-hotcell-server/test/fixtures/protected.pdf b/activestorage-hotcell-server/test/fixtures/protected.pdf new file mode 100644 index 0000000000000000000000000000000000000000..0c23ffd1d1bfc5b3f75871e384b4fc9bb202fae5 GIT binary patch literal 2872 zcmbVOONeAw8D87yCUFp5ghRA7KBnuuAJU9WzhdKzJ$6kx$|S_|_|HrY-F2I)+e44h zEJR5_$imA5a%o|C@~{xcv_Ye8 zhxKsukmM=4M4QaB6DPV~?@T;TW2T^Uo%POM-X_*tjBVU(&SQIqhNUwl51Bl<(_Uys zY~BV}xiwz<5~@y73m;vgWQ?jx=4ic8#fw;c=0wpNu6*S==VZAKy40+VStp9Ns%T|W;9iv&xG^z1&vW2OdNRDEk~HTjI9r9cGA7B5tyKh} zjVTDRQa0XYV`bGkdTyr%yX1I8VngCw%4~}E!8zrfiBiGWT12+WDpO$22r5R($TNvd zN=n!aIS8JTM52hNtX!%taVNP@JUOW3N_MVT?_4OI5s?HTFF^(4baq;b;58!3GRUA* zgoLZHWM4AE5E;InZpvO>l(WtiVKilFEO8zXTPebM3Y?U;BFLl=M$*n3t)Wl)h}?-p zQifP6UT``Y1>1(S6E#$F(3Arz-8I>$DLa|Eu8}oo&4cGMYL%r!pjl}pOsELUWi6FN z4{4g=NYsHOA89kGY<{r*g?=7#cD#1gHC{@nr7f;?Qv_ZoI!s#+Y8MulyH27lXIR4Vdf-*y38b6Te^w5hKPgLZs|MvKkXb~Eg9YG<5&>gxq)Kr{8$% zBVYUI=Re2Zb=Tcb{Nd^GL!Z8BwtvI#|M}ogUJTD%KKb^gJ5H^C^2+_szx1hZ|KmqL zK9Yd0ME_T4=+r%b@v!f%lCS3OJKuEoBfos~FMoRaEZ(YE&h_1ue4=OczpT_e)yY;P8SD$=& z?v1yKjd%R~$`?Oz$FF|IKK%8U-u$|UpZnX&^Pl-`?}%ICFxf7yOlF|pmi?!W3YRZ+ zz0q50^4d0mY!_EfgNK*<8|rYn_NQf9<9-{5C5{l|zUu&9)>{Kdi42UZ666BR)eekW z44_*QY0yu|HK!UK*j0N|D&Bm^Hizs%lg{X1Y_U0q1S-rjYjOZZZ}FuPxJZDvHQW%K z^hwkLCR2&Hb=H@{0|2wy8l{Le7cIfyNr8yKHQCExxd4@mjv_(_>FhYB7z45e&=8tr zj2`lycudJxlahmHss^xWh5S0;q}Ok^FnfCm#NX)P@gq zMQRPJ?An?D4t5_lytW25Ke7hLuU`{fq6`21tdIi4aNU{$j2v0>K)&nbEI;ClaKPm2 z<-9oROE?@yJBowNj@3aLtHau`?w*Ki error - refuse "unreadable", error, timing + cause = error.class.respond_to?(:cause) ? error.class.cause : nil + refuse "unreadable", error, timing, cause: cause end # `post` returns what each output received and the worker used to throw all of it away, leaving the From 77061b36d6ce90c49892d600aca9de5cae4c706d Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Tue, 6 Oct 2026 13:07:01 -0400 Subject: [PATCH 2/3] Document the `protected` cause for `unreadable` `PROTECTED` sat among the `killed` causes in `codes.rb`, under their shared comment, and no doc mentioned that `unreadable` can carry a cause. Move the constant beside `KILLED` with its own comment, document the cause in `docs/codes.md`, `docs/operation-api.md` and `docs/active-storage.md`, and add a changelog entry. --- CHANGELOG.md | 12 ++++++++++++ docs/active-storage.md | 2 ++ docs/codes.md | 16 +++++++++++++++- docs/operation-api.md | 4 ++++ hotcell-core/lib/hot_cell/codes.rb | 7 ++++--- 5 files changed, 37 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d17346..5ca1389 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ gem but are what an operator runs against their own image. ## next / unreleased +### HotCell::Server + +#### Added + +* An operation can say why it couldn't decode an input: a subclass of `HotCell::UnreadableInput` that defines `self.cause` sends that cause with the `unreadable` failure. `HotCell::Codes::PROTECTED` names the first such cause, `protected`. See `docs/codes.md`. + ### ActiveStorage::HotCell::Client #### Fixed @@ -27,6 +33,12 @@ gem but are what an operator runs against their own image. * `hotcell-client-legacy` is a new gem that lets an application on a legacy Ruby version call a cell. It has no dependencies, and it speaks the same wire protocol as `hotcell-client` but leaves out Rails integration, Active Storage support and metrics. See [docs/client-legacy.md](docs/client-legacy.md). (#100) +### ActiveStorage::HotCell::Server + +#### Added + +* The `mutool` PDF previewer answers `unreadable` with cause `protected` for a password-protected PDF. An application that shows a lock placeholder for these PDFs can read the cause instead of matching `mutool`'s error message. + ## v1.0.0 / 2026-10-05 ### Documentation diff --git a/docs/active-storage.md b/docs/active-storage.md index 0934313..40e5d02 100644 --- a/docs/active-storage.md +++ b/docs/active-storage.md @@ -85,6 +85,8 @@ The client raises the cell's `permanent:` or `transient:` class. See [Response c `ActiveStorage::CreateVariantsJob`, `ActiveStorage::PreviewImageJob`, and `ActiveStorage::TransformJob`, for each one that this Rails version has. Each registered cell that a shipped client names contributes its transient class. A cell that isn't registered yet contributes nothing. +- **`Previewers::Pdf::Mutool`** answers `unreadable` with cause `protected` for a password-protected PDF. + See [Causes of `unreadable`](codes.md#causes-of-unreadable). ## Limits diff --git a/docs/codes.md b/docs/codes.md index 3e94557..7c7688f 100644 --- a/docs/codes.md +++ b/docs/codes.md @@ -99,9 +99,21 @@ that the supervisor can't see: a cgroup out-of-memory kill chosen on aggregate p signalling another, which nothing prevents because workers share a uid. The worker that holds the request decides `memory` and `fsize` itself. +## Causes of `unreadable` + +An `unreadable` failure is permanent whatever its cause. Most carry no cause. An operation that can tell +why it couldn't decode the input sends one, so that a caller can act on the reason without parsing the +message: + +- `protected`: the document demands a password that the cell doesn't have. The shipped `mutool` PDF + previewer sends it for a password-protected PDF. + +An operation sends a cause by raising a subclass of `HotCell::UnreadableInput` that defines `self.cause`. +See [`unreadable(*classes)`](operation-api.md#unreadableclasses). + ## Unknown codes and causes -A code or cause that a client doesn't recognize isn't permanent: +An unknown code, or an unknown cause of `killed`, isn't permanent: - The `permanent` flag travels on the wire, set by the side that knows. A client that's older than a code still disposes of that code correctly. @@ -109,6 +121,8 @@ A code or cause that a client doesn't recognize isn't permanent: - An unknown `killed` cause is transient. Adding a kill reason to the supervisor without a row in `Codes::PERMANENT_BY_CAUSE` can't make it permanent. +A cause of `unreadable` doesn't change its permanence, so an unknown one is still permanent. + Retrying something permanent costs some work. Recording something transient is irreversible. **Caution:** A compromised cell can forge the split. The client believes a boolean `permanent` from the diff --git a/docs/operation-api.md b/docs/operation-api.md index c18e22f..f11d6fb 100644 --- a/docs/operation-api.md +++ b/docs/operation-api.md @@ -146,6 +146,10 @@ broke". When `perform` raises one of them, the cell answers `unreadable`, which [permanent](codes.md). `HotCell::UnreadableInput` is always included. A subclass adds to the classes that its ancestors declared. +To tell the caller why the input can't be decoded, raise a subclass of `HotCell::UnreadableInput` that +defines `self.cause`. The cell sends the cause with the `unreadable` failure. See +[Causes of `unreadable`](codes.md#causes-of-unreadable). + ### `abstract_operation` Marks a class that exists to be inherited from, not to be dispatched to. diff --git a/hotcell-core/lib/hot_cell/codes.rb b/hotcell-core/lib/hot_cell/codes.rb index ced3a6e..106c0c7 100644 --- a/hotcell-core/lib/hot_cell/codes.rb +++ b/hotcell-core/lib/hot_cell/codes.rb @@ -70,9 +70,6 @@ module Codes # neither `reap` nor `drain_signals` nor `run` catches it. A typo would take the whole cell down from the # one path whose job is reporting a dead worker. FSIZE = "fsize" - - # A cause for `unreadable`: the document demands credentials the cell does not have. - PROTECTED = "protected" MEMORY = "memory" DEADLINE = "deadline" CRASHED = "crashed" @@ -103,6 +100,10 @@ module Codes KILLED = "killed" + # A cause for `unreadable`, which is permanent whatever its cause: the document demands credentials the + # cell does not have. It lets a caller show a locked document differently from a broken one. + PROTECTED = "protected" + class << self def permanent?(code, cause: nil) code = code.to_s From 693173269b3926bbff851451ee719b52bcd1f49c Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Wed, 7 Oct 2026 17:10:07 -0400 Subject: [PATCH 3/3] Send cause `protected` from the Poppler previewer The Poppler previewer answered a password-protected PDF as `unreadable` with no cause, unlike the `mutool` previewer. Map `pdftoppm`'s `Incorrect password` stderr to `ProtectedDocument`. --- CHANGELOG.md | 2 +- .../hot_cell/server/previewers/pdf/poppler.rb | 2 +- .../test/previewers_test.rb | 14 ++++++++++++++ docs/active-storage.md | 4 ++-- docs/codes.md | 4 ++-- 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ca1389..a350a28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,7 @@ gem but are what an operator runs against their own image. #### Added -* The `mutool` PDF previewer answers `unreadable` with cause `protected` for a password-protected PDF. An application that shows a lock placeholder for these PDFs can read the cause instead of matching `mutool`'s error message. +* The `mutool` and Poppler PDF previewers answer `unreadable` with cause `protected` for a password-protected PDF. An application that shows a lock placeholder for these PDFs can read the cause instead of matching the tool's error message. ## v1.0.0 / 2026-10-05 diff --git a/activestorage-hotcell-server/lib/active_storage/hot_cell/server/previewers/pdf/poppler.rb b/activestorage-hotcell-server/lib/active_storage/hot_cell/server/previewers/pdf/poppler.rb index 59b7489..4695344 100644 --- a/activestorage-hotcell-server/lib/active_storage/hot_cell/server/previewers/pdf/poppler.rb +++ b/activestorage-hotcell-server/lib/active_storage/hot_cell/server/previewers/pdf/poppler.rb @@ -22,7 +22,7 @@ class Poppler < Pdf def render(source, destination, page:, resolution:) run! "pdftoppm", "-png", "-singlefile", "-cropbox", "-r", resolution.to_s, "-f", page.to_s, "-l", page.to_s, source.fd_path, destination.path, - pass: [ source.to_io ] + pass: [ source.to_io ], causes: { /Incorrect password/ => ProtectedDocument } destination.adopt destination.path(extension: "png") end diff --git a/activestorage-hotcell-server/test/previewers_test.rb b/activestorage-hotcell-server/test/previewers_test.rb index 49b1d31..eaf23ff 100644 --- a/activestorage-hotcell-server/test/previewers_test.rb +++ b/activestorage-hotcell-server/test/previewers_test.rb @@ -188,11 +188,25 @@ def test_poppler_on_something_that_is_not_a_pdf_is_unreadable outputs: [ destination ]) assert_predicate failure, :permanent? + assert_nil failure.cause assert_match "pdftoppm", failure.message end end end + def test_poppler_on_a_password_protected_pdf_answers_the_protected_cause + Cell.boot do |cell| + with_output(".png") do |destination| + failure = assert_failed "unreadable", cell.call("active_storage.previewers.pdf.poppler", + inputs: [ fixture("protected.pdf") ], + outputs: [ destination ]) + + assert_equal HotCell::Codes::PROTECTED, failure.cause + assert_predicate failure, :permanent? + end + end + end + # Rails' default preview arguments choose the first keyframe or scene change rather than the literal first # frame, because so many videos open on black. A drop-in replacement has to keep that choice: fade_in.mp4 # opens on half a second of black before a hard cut, and its preview must come from after the cut. diff --git a/docs/active-storage.md b/docs/active-storage.md index 40e5d02..731f3ce 100644 --- a/docs/active-storage.md +++ b/docs/active-storage.md @@ -85,8 +85,8 @@ The client raises the cell's `permanent:` or `transient:` class. See [Response c `ActiveStorage::CreateVariantsJob`, `ActiveStorage::PreviewImageJob`, and `ActiveStorage::TransformJob`, for each one that this Rails version has. Each registered cell that a shipped client names contributes its transient class. A cell that isn't registered yet contributes nothing. -- **`Previewers::Pdf::Mutool`** answers `unreadable` with cause `protected` for a password-protected PDF. - See [Causes of `unreadable`](codes.md#causes-of-unreadable). +- **`Previewers::Pdf::Mutool`** and **`Previewers::Pdf::Poppler`** answer `unreadable` with cause `protected` + for a password-protected PDF. See [Causes of `unreadable`](codes.md#causes-of-unreadable). ## Limits diff --git a/docs/codes.md b/docs/codes.md index 7c7688f..80d05c5 100644 --- a/docs/codes.md +++ b/docs/codes.md @@ -105,8 +105,8 @@ An `unreadable` failure is permanent whatever its cause. Most carry no cause. An why it couldn't decode the input sends one, so that a caller can act on the reason without parsing the message: -- `protected`: the document demands a password that the cell doesn't have. The shipped `mutool` PDF - previewer sends it for a password-protected PDF. +- `protected`: the document demands a password that the cell doesn't have. The shipped `mutool` and Poppler PDF + previewers send it for a password-protected PDF. An operation sends a cause by raising a subclass of `HotCell::UnreadableInput` that defines `self.cause`. See [`unreadable(*classes)`](operation-api.md#unreadableclasses).