diff --git a/app/controllers/action_text/markdown/uploads_controller.rb b/app/controllers/action_text/markdown/uploads_controller.rb index f7d4292a..3da17c36 100644 --- a/app/controllers/action_text/markdown/uploads_controller.rb +++ b/app/controllers/action_text/markdown/uploads_controller.rb @@ -51,8 +51,10 @@ def set_attachment # An unpublished book's uploads are as private as the book itself. Serving them to # anyone holding the URL made this a way to read them without an access row, and - # caching them publicly for a year put them in shared caches besides. + # caching them publicly for a year put them in shared caches besides. An attachment + # whose owning book can't be resolved has no book to authorize against, so fail + # closed rather than letting the nil short-circuit the check. def ensure_attachment_readable - head :not_found unless @book.nil? || @book.published? || @book.accessable? + head :not_found unless @book&.published? || @book&.accessable? end end diff --git a/test/controllers/action_text/markdown/uploads_controller_test.rb b/test/controllers/action_text/markdown/uploads_controller_test.rb index 93472f32..029cd30d 100644 --- a/test/controllers/action_text/markdown/uploads_controller_test.rb +++ b/test/controllers/action_text/markdown/uploads_controller_test.rb @@ -131,6 +131,21 @@ class ActionText::Markdown::UploadsControllerTest < ActionDispatch::IntegrationT assert_response :not_found end + test "an attachment whose owning book can't be resolved is not served" do + books(:handbook).update! published: false + attachment = attach_upload_to_welcome_page + + # Sever the leaf without cascading the destroy, leaving the attachment live but + # its owning book unresolvable. The guard must fail closed on the nil book. + pages(:welcome).leaf.delete + assert_nil pages(:welcome).reload.owning_book + + reset! + get action_text_markdown_upload_path(slug: attachment.slug) + + assert_response :not_found + end + test "an attachment of an unpublished book is served to a reader, but not publicly cached" do books(:handbook).update! published: false attachment = attach_upload_to_welcome_page