diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d17346..a350a28 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` 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 ### Documentation 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/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/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 0000000..0c23ffd Binary files /dev/null and b/activestorage-hotcell-server/test/fixtures/protected.pdf differ diff --git a/activestorage-hotcell-server/test/previewers_test.rb b/activestorage-hotcell-server/test/previewers_test.rb index f85b584..eaf23ff 100644 --- a/activestorage-hotcell-server/test/previewers_test.rb +++ b/activestorage-hotcell-server/test/previewers_test.rb @@ -94,11 +94,25 @@ def test_something_that_is_not_a_pdf_is_unreadable outputs: [ destination ]) assert_predicate failure, :permanent? + assert_nil failure.cause assert_match "mutool", failure.message end end end + def test_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.mutool", + inputs: [ fixture("protected.pdf") ], + outputs: [ destination ]) + + assert_equal HotCell::Codes::PROTECTED, failure.cause + assert_predicate failure, :permanent? + end + end + end + # JPEG rather than PNG, because Rails' own video previewer yields image/jpeg and this drops into its place. def test_a_video_preview_comes_back_as_the_jpeg_rails_produces Cell.boot do |cell| @@ -174,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 0934313..731f3ce 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`** 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 3e94557..80d05c5 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` 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). + ## 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 350576f..106c0c7 100644 --- a/hotcell-core/lib/hot_cell/codes.rb +++ b/hotcell-core/lib/hot_cell/codes.rb @@ -100,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 diff --git a/hotcell-server/lib/hot_cell/server/errors.rb b/hotcell-server/lib/hot_cell/server/errors.rb index 71ae1d9..a79813d 100644 --- a/hotcell-server/lib/hot_cell/server/errors.rb +++ b/hotcell-server/lib/hot_cell/server/errors.rb @@ -7,8 +7,13 @@ class OperationError < StandardError; end # The input could not be decoded. Terminal: the same bytes fail the same way on an idle cell. Common # rather than exceptional — it covers truncated uploads, formats the build was not compiled with, and - # formats deliberately refused. - class UnreadableInput < OperationError; end + # formats deliberately refused. A subclass may name the failure's cause, for a caller that must act + # on the reason without parsing a message. + class UnreadableInput < OperationError + def self.cause + nil + end + end # Reported as `killed` with `cause: memory` rather than as an ordinary failure, because it is the # decompression-bomb case and a caller must be able to act on it without parsing a message. diff --git a/hotcell-server/lib/hot_cell/worker.rb b/hotcell-server/lib/hot_cell/worker.rb index fd188b4..732e48e 100644 --- a/hotcell-server/lib/hot_cell/worker.rb +++ b/hotcell-server/lib/hot_cell/worker.rb @@ -219,7 +219,8 @@ def perform(operation, inputs, outputs, payload, timing) response rescue *operation.unreadable => 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