From f4c9d2b0492d5a2dc3de052352f7a2e90d150f3f Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 13 Mar 2026 13:35:28 +0000 Subject: [PATCH 1/2] Exports: Fixed scope of pages in chapter MD export Added tests to cover children of all MD exports --- app/Exports/ExportFormatter.php | 2 +- tests/Exports/MarkdownExportTest.php | 46 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/app/Exports/ExportFormatter.php b/app/Exports/ExportFormatter.php index ad489aba1..c5973eace 100644 --- a/app/Exports/ExportFormatter.php +++ b/app/Exports/ExportFormatter.php @@ -323,7 +323,7 @@ class ExportFormatter $text .= $description . "\n\n"; } - foreach ($chapter->pages as $page) { + foreach ($chapter->getVisiblePages() as $page) { $text .= $this->pageToMarkdown($page) . "\n\n"; } diff --git a/tests/Exports/MarkdownExportTest.php b/tests/Exports/MarkdownExportTest.php index 6bf585d59..09928ced2 100644 --- a/tests/Exports/MarkdownExportTest.php +++ b/tests/Exports/MarkdownExportTest.php @@ -56,6 +56,20 @@ class MarkdownExportTest extends TestCase $resp->assertSee('My **chapter** description'); } + public function test_chapter_markdown_export_pages_are_permission_controlled() + { + $chapter = $this->entities->chapterHasPages(); + $page = $chapter->pages()->first(); + $page->name = 'MyPageWhichShouldNotBeFound'; + $page->save(); + $this->permissions->disableEntityInheritedPermissions($page); + + $resp = $this->asEditor()->get($chapter->getUrl('/export/markdown')); + + $resp->assertSee('# ' . $chapter->name); + $resp->assertDontSee('MyPageWhichShouldNotBeFound'); + } + public function test_book_markdown_export() { $book = Book::query()->whereHas('pages')->whereHas('chapters')->first(); @@ -76,6 +90,38 @@ class MarkdownExportTest extends TestCase $resp->assertSee('My **chapter** description'); } + public function test_book_markdown_export_chapters_are_permission_controlled() + { + $book = $this->entities->bookHasChaptersAndPages(); + $chapter = $book->chapters()->first(); + $page = $chapter->pages()->first(); + $page->name = 'MyPageWhichShouldNotBeFound'; + $page->save(); + $chapter->name = 'MyChapterWhichShouldNotBeFound'; + $chapter->save(); + $this->permissions->disableEntityInheritedPermissions($chapter); + + $resp = $this->asEditor()->get($book->getUrl('/export/markdown')); + + $resp->assertSee('# ' . $book->name); + $resp->assertDontSee('MyChapterWhichShouldNotBeFound'); + $resp->assertDontSee('MyPageWhichShouldNotBeFound'); + } + + public function test_book_markdown_export_direct_pages_are_permission_controlled() + { + $book = $this->entities->bookHasChaptersAndPages(); + $page = $book->directPages()->first(); + $page->name = 'MyPageWhichShouldNotBeFound'; + $page->save(); + $this->permissions->disableEntityInheritedPermissions($page); + + $resp = $this->asEditor()->get($book->getUrl('/export/markdown')); + + $resp->assertSee('# ' . $book->name); + $resp->assertDontSee('MyPageWhichShouldNotBeFound'); + } + public function test_book_markdown_export_concats_immediate_pages_with_newlines() { /** @var Book $book */ From a9ffd3e0c77019916bba21830d8514cca723ce17 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 16 Mar 2026 18:28:44 +0000 Subject: [PATCH 2/2] Responses: Added extra sanitization for download names From testing, don't think this could exploited directly, as the response would error instead of allowing control characters, but this adds an extra layer of sanitization, and switches to encoded disposition filenames for better UTF8 support. --- app/Http/DownloadResponseFactory.php | 7 +++++-- tests/Api/ExportsApiTest.php | 30 ++++++++++++++-------------- tests/Exports/HtmlExportTest.php | 6 +++--- tests/Exports/MarkdownExportTest.php | 2 +- tests/Exports/PdfExportTest.php | 6 +++--- tests/Exports/TextExportTest.php | 6 +++--- tests/Uploads/AttachmentTest.php | 19 ++++++++++++++++-- 7 files changed, 47 insertions(+), 29 deletions(-) diff --git a/app/Http/DownloadResponseFactory.php b/app/Http/DownloadResponseFactory.php index 8384484ad..1b6256c00 100644 --- a/app/Http/DownloadResponseFactory.php +++ b/app/Http/DownloadResponseFactory.php @@ -102,12 +102,15 @@ class DownloadResponseFactory protected function getHeaders(string $fileName, int $fileSize, string $mime = 'application/octet-stream'): array { $disposition = ($mime === 'application/octet-stream') ? 'attachment' : 'inline'; - $downloadName = str_replace('"', '', $fileName); + + $downloadName = str_replace(['"', '/', '\\', '$'], '', $fileName); + $downloadName = preg_replace('/[\x00-\x1F\x7F]/', '', $downloadName); + $encodedDownloadName = rawurlencode($downloadName); return [ 'Content-Type' => $mime, 'Content-Length' => $fileSize, - 'Content-Disposition' => "{$disposition}; filename=\"{$downloadName}\"", + 'Content-Disposition' => "{$disposition}; filename*=UTF-8''{$encodedDownloadName}", 'X-Content-Type-Options' => 'nosniff', ]; } diff --git a/tests/Api/ExportsApiTest.php b/tests/Api/ExportsApiTest.php index e1ac698d0..7951e04d2 100644 --- a/tests/Api/ExportsApiTest.php +++ b/tests/Api/ExportsApiTest.php @@ -19,7 +19,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/books/{$book->id}/export/html"); $resp->assertStatus(200); $resp->assertSee($book->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.html"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.html'); } public function test_book_plain_text_endpoint() @@ -30,7 +30,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/books/{$book->id}/export/plaintext"); $resp->assertStatus(200); $resp->assertSee($book->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.txt"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.txt'); } public function test_book_pdf_endpoint() @@ -40,7 +40,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/books/{$book->id}/export/pdf"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.pdf"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.pdf'); } public function test_book_markdown_endpoint() @@ -50,7 +50,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/books/{$book->id}/export/markdown"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.md"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.md'); $resp->assertSee('# ' . $book->name); $resp->assertSee('# ' . $book->pages()->first()->name); $resp->assertSee('# ' . $book->chapters()->first()->name); @@ -63,7 +63,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/books/{$book->id}/export/zip"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.zip"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.zip'); $zip = ZipTestHelper::extractFromZipResponse($resp); $this->assertArrayHasKey('book', $zip->data); @@ -77,7 +77,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/chapters/{$chapter->id}/export/html"); $resp->assertStatus(200); $resp->assertSee($chapter->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.html"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.html'); } public function test_chapter_plain_text_endpoint() @@ -88,7 +88,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/chapters/{$chapter->id}/export/plaintext"); $resp->assertStatus(200); $resp->assertSee($chapter->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.txt"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.txt'); } public function test_chapter_pdf_endpoint() @@ -98,7 +98,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/chapters/{$chapter->id}/export/pdf"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.pdf"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.pdf'); } public function test_chapter_markdown_endpoint() @@ -108,7 +108,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/chapters/{$chapter->id}/export/markdown"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.md"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.md'); $resp->assertSee('# ' . $chapter->name); $resp->assertSee('# ' . $chapter->pages()->first()->name); } @@ -120,7 +120,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/chapters/{$chapter->id}/export/zip"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.zip"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.zip'); $zip = ZipTestHelper::extractFromZipResponse($resp); $this->assertArrayHasKey('chapter', $zip->data); @@ -134,7 +134,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/pages/{$page->id}/export/html"); $resp->assertStatus(200); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.html"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.html'); } public function test_page_plain_text_endpoint() @@ -145,7 +145,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/pages/{$page->id}/export/plaintext"); $resp->assertStatus(200); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.txt"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.txt'); } public function test_page_pdf_endpoint() @@ -155,7 +155,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/pages/{$page->id}/export/pdf"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.pdf"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.pdf'); } public function test_page_markdown_endpoint() @@ -166,7 +166,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/pages/{$page->id}/export/markdown"); $resp->assertStatus(200); $resp->assertSee('# ' . $page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.md"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.md'); } public function test_page_zip_endpoint() @@ -176,7 +176,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/pages/{$page->id}/export/zip"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.zip"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.zip'); $zip = ZipTestHelper::extractFromZipResponse($resp); $this->assertArrayHasKey('page', $zip->data); diff --git a/tests/Exports/HtmlExportTest.php b/tests/Exports/HtmlExportTest.php index e039fb2cc..f23352e0e 100644 --- a/tests/Exports/HtmlExportTest.php +++ b/tests/Exports/HtmlExportTest.php @@ -18,7 +18,7 @@ class HtmlExportTest extends TestCase $resp = $this->get($page->getUrl('/export/html')); $resp->assertStatus(200); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.html"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.html'); } public function test_book_html_export() @@ -31,7 +31,7 @@ class HtmlExportTest extends TestCase $resp->assertStatus(200); $resp->assertSee($book->name); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.html"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.html'); } public function test_book_html_export_shows_html_descriptions() @@ -58,7 +58,7 @@ class HtmlExportTest extends TestCase $resp->assertStatus(200); $resp->assertSee($chapter->name); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.html"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.html'); } public function test_chapter_html_export_shows_html_descriptions() diff --git a/tests/Exports/MarkdownExportTest.php b/tests/Exports/MarkdownExportTest.php index 09928ced2..ac1e283f3 100644 --- a/tests/Exports/MarkdownExportTest.php +++ b/tests/Exports/MarkdownExportTest.php @@ -14,7 +14,7 @@ class MarkdownExportTest extends TestCase $resp = $this->asEditor()->get($page->getUrl('/export/markdown')); $resp->assertStatus(200); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.md"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.md'); } public function test_page_markdown_export_uses_existing_markdown_if_apparent() diff --git a/tests/Exports/PdfExportTest.php b/tests/Exports/PdfExportTest.php index e4de87d0d..f311f8457 100644 --- a/tests/Exports/PdfExportTest.php +++ b/tests/Exports/PdfExportTest.php @@ -17,7 +17,7 @@ class PdfExportTest extends TestCase $resp = $this->get($page->getUrl('/export/pdf')); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.pdf"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.pdf'); } public function test_book_pdf_export() @@ -28,7 +28,7 @@ class PdfExportTest extends TestCase $resp = $this->get($book->getUrl('/export/pdf')); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.pdf"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.pdf'); } public function test_chapter_pdf_export() @@ -38,7 +38,7 @@ class PdfExportTest extends TestCase $resp = $this->get($chapter->getUrl('/export/pdf')); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.pdf"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.pdf'); } diff --git a/tests/Exports/TextExportTest.php b/tests/Exports/TextExportTest.php index c593a6585..4b2d62887 100644 --- a/tests/Exports/TextExportTest.php +++ b/tests/Exports/TextExportTest.php @@ -14,7 +14,7 @@ class TextExportTest extends TestCase $resp = $this->get($page->getUrl('/export/plaintext')); $resp->assertStatus(200); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.txt"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.txt'); } public function test_book_text_export() @@ -35,7 +35,7 @@ class TextExportTest extends TestCase $resp->assertSee($directPage->name); $resp->assertSee('My awesome page'); $resp->assertSee('My little nested page'); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.txt"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.txt'); } public function test_book_text_export_format() @@ -68,7 +68,7 @@ class TextExportTest extends TestCase $resp->assertSee($chapter->name); $resp->assertSee($page->name); $resp->assertSee('This is content within the page!'); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.txt"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.txt'); } public function test_chapter_text_export_format() diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php index b443ca9f1..945fc258d 100644 --- a/tests/Uploads/AttachmentTest.php +++ b/tests/Uploads/AttachmentTest.php @@ -324,7 +324,7 @@ class AttachmentTest extends TestCase $attachmentGet = $this->get($attachment->getUrl(true)); // http-foundation/Response does some 'fixing' of responses to add charsets to text responses. $attachmentGet->assertHeader('Content-Type', 'text/plain; charset=utf-8'); - $attachmentGet->assertHeader('Content-Disposition', 'inline; filename="upload_test_file.txt"'); + $attachmentGet->assertHeader('Content-Disposition', 'inline; filename*=UTF-8\'\'upload_test_file.txt'); $attachmentGet->assertHeader('X-Content-Type-Options', 'nosniff'); $this->files->deleteAllAttachmentFiles(); @@ -340,7 +340,22 @@ class AttachmentTest extends TestCase $attachmentGet = $this->get($attachment->getUrl(true)); // http-foundation/Response does some 'fixing' of responses to add charsets to text responses. $attachmentGet->assertHeader('Content-Type', 'text/plain; charset=utf-8'); - $attachmentGet->assertHeader('Content-Disposition', 'inline; filename="test_file.html"'); + $attachmentGet->assertHeader('Content-Disposition', 'inline; filename*=UTF-8\'\'test_file.html'); + + $this->files->deleteAllAttachmentFiles(); + } + + public function test_file_access_name_in_content_disposition_header_is_sanitized() + { + $page = $this->entities->page(); + $this->asAdmin(); + + $attachment = $this->files->uploadAttachmentDataToPage($this, $page, 'test_file.html', '

testing

', 'text/html'); + $attachment->name = "my\\_/super\n_fu\$n_\tfile"; + $attachment->save(); + + $attachmentGet = $this->get($attachment->getUrl(true)); + $attachmentGet->assertHeader('Content-Disposition', 'inline; filename*=UTF-8\'\'my_super_fun_file.html'); $this->files->deleteAllAttachmentFiles(); }