diff --git a/README.md b/README.md index f17ced6..43277ab 100644 --- a/README.md +++ b/README.md @@ -142,9 +142,23 @@ npm ci && npm run build Point your web server at `public/`, then open **`https://your-domain/install`**. The wizard checks the server -requirements, then asks for the site URL, your name, email and password, -and installs. It only exists while Docket has no user account: once your -login is created, `/install` permanently redirects to the login screen. +requirements, then asks for a setup code, the site URL, your name, email +and password, and installs. + +The setup code proves you control the server: anyone can find a fresh +deployment's `/install`, so it won't create the account without it. Read +it on the server once the setup page has loaded: + +```sh +cat storage/app/install/setup-code +``` + +The installer only exists while Docket has no account. Once your login +is created it leaves a marker (`storage/app/install/installed`) and +`/install` permanently redirects to the login screen. If the database +can't be read, it answers 503 rather than treating the site as fresh. +Docket has exactly one account: neither installer, nor `db:seed`, will +create a second one. ### Console installer @@ -191,6 +205,10 @@ Development server (app + queue + logs + Vite): `composer dev`. User=www-data ``` + Keep `--timeout` below the queue's `retry_after` (360 seconds by + default, `DB_QUEUE_RETRY_AFTER`), or a slow fetch is handed to a second + worker while the first is still running it. + 5. One cron entry drives the scheduler (backups, cleanup, monitor): ``` @@ -218,18 +236,38 @@ BACKUP_ENDPOINT=https://.r2.cloudflarestorage.com BACKUP_REGION=auto ``` -Test a restore early: +The database in the archive is an SQL dump (`db-dumps/*.sql`, written by +`sqlite3 .dump`), not a database file: it has to be imported into a new +database. Test a restore early, on a copy of the server if you can: ```sh php artisan backup:run # download the newest zip from the bucket, then: unzip Docket/….zip -d /tmp/restore -mv /tmp/restore/db-dumps/database.sqlite database/database.sqlite +ls /tmp/restore/db-dumps/ # one .sql file + +# Build a new database from the dump and check it. +sqlite3 /tmp/restore/restored.sqlite < /tmp/restore/db-dumps/*.sql +sqlite3 /tmp/restore/restored.sqlite "PRAGMA integrity_check;" # expect: ok +sqlite3 /tmp/restore/restored.sqlite "SELECT count(*) FROM items;" +sqlite3 /tmp/restore/restored.sqlite "INSERT INTO items_fts(items_fts) VALUES('rebuild');" + +# Swap it in with nothing writing: stop the worker and the site first. +php artisan down +sudo systemctl stop docket-worker # whatever runs queue:work +mkdir -p /tmp/before-restore && cp database/database.sqlite* /tmp/before-restore/ +mv /tmp/restore/restored.sqlite database/database.sqlite +rm -f database/database.sqlite-wal database/database.sqlite-shm rsync -a /tmp/restore/…/storage/app/private/ storage/app/private/ -php artisan migrate:status +chown -R www-data: database storage/app/private +php artisan migrate --force # if the backup predates a release +php artisan up +sudo systemctl start docket-worker ``` -Open a saved article and check its images load. +Then open a saved article, check its images load, and search for a word +you know is in it. The originals stay in `/tmp/before-restore` until +you're satisfied. ## Security notes diff --git a/app/Console/Commands/DocketInstallCommand.php b/app/Console/Commands/DocketInstallCommand.php index 63bfb18..26983e7 100644 --- a/app/Console/Commands/DocketInstallCommand.php +++ b/app/Console/Commands/DocketInstallCommand.php @@ -2,6 +2,7 @@ namespace App\Console\Commands; +use App\Exceptions\InstallStateUnknownException; use App\Services\Installer; use Illuminate\Console\Attributes\Description; use Illuminate\Console\Attributes\Signature; @@ -41,7 +42,15 @@ public function handle(Installer $installer): int return SymfonyCommand::FAILURE; } - if ($installer->isInstalled() && ! $this->option('force')) { + try { + $installed = $installer->isInstalled(); + } catch (InstallStateUnknownException $e) { + $this->components->error($e->getMessage()); + + return SymfonyCommand::FAILURE; + } + + if ($installed && ! $this->option('force')) { $this->components->warn( 'Docket already has a user account. Re-run with --force to run the installer anyway (existing data is never deleted).' ); @@ -95,32 +104,51 @@ public function handle(Installer $installer): int return SymfonyCommand::FAILURE; } - // Persist the identity to .env so a later `db:seed` recreates the - // same account rather than the config/docket.php fallback owner. - // The password is deliberately NOT written to .env: it is hashed - // straight into the database below. - $installer->writeEnvValues([ - 'APP_URL' => $url, - 'DOCKET_USER_NAME' => $name, - 'DOCKET_USER_EMAIL' => $email, - ]); - $this->components->task('.env updated (APP_URL, owner name/email)', fn (): bool => true); - - $installer->ensureSqliteDatabase(); - - $this->components->task('running database migrations', fn (): bool => $installer->migrate()); - - if ($installer->ownerExists($email)) { - $this->components->warn("An account for {$email} already exists — its password was left unchanged."); - } else { - $this->components->task( - "creating owner account for {$email}", - function () use ($installer, $name, $email, $plainPassword): bool { - $installer->createOwner($name, $email, $plainPassword); - - return true; - } - ); + // Serialised with the web installer, so the two can't race each + // other into creating two accounts. + $lock = $installer->acquireLock(); + + if ($lock === null) { + $this->components->error('Another installation is running right now. Try again in a moment.'); + + return SymfonyCommand::FAILURE; + } + + try { + $installer->ensureSqliteDatabase(); + + $this->components->task('running database migrations', fn (): bool => $installer->migrate()); + + if ($installer->ownerExists($email)) { + $this->components->warn("An account for {$email} already exists — its password was left unchanged."); + } elseif ($installer->hasOwner()) { + $this->components->warn('Docket already has its owner account, under a different email. It has one account, so no new one was created; change the email on the Profile page instead.'); + + return SymfonyCommand::FAILURE; + } else { + $this->components->task( + "creating owner account for {$email}", + function () use ($installer, $name, $email, $plainPassword): bool { + $installer->createOwner($name, $email, $plainPassword); + + return true; + } + ); + } + + // Written once the account is in place, so a refused install + // leaves .env alone. The password is deliberately NOT written + // to .env: it is hashed straight into the database. + $installer->writeEnvValues([ + 'APP_URL' => $url, + 'DOCKET_USER_NAME' => $name, + 'DOCKET_USER_EMAIL' => $email, + ]); + $this->components->task('.env updated (APP_URL, owner name/email)', fn (): bool => true); + + $installer->markInstalled(); + } finally { + $installer->releaseLock($lock); } $this->printNextSteps($url, $email); diff --git a/app/Console/Commands/DocketRefetchCommand.php b/app/Console/Commands/DocketRefetchCommand.php index 8573dcf..ab6a44e 100644 --- a/app/Console/Commands/DocketRefetchCommand.php +++ b/app/Console/Commands/DocketRefetchCommand.php @@ -42,7 +42,10 @@ public function handle(): int $spacing = max(0, (int) settings('import.spacing_seconds')); $count = 0; - foreach ($query->orderBy('id')->lazy() as $item) { + // By ID rather than by offset: items leave the failed/pending + // selection as they are fetched (at once, on a sync queue), and + // offset paging over a shrinking result set skips rows. + foreach ($query->withoutBodies()->lazyById(200, 'items.id', 'id') as $item) { $this->queue($item, $count * $spacing); $count++; } diff --git a/app/Exceptions/InstallStateUnknownException.php b/app/Exceptions/InstallStateUnknownException.php new file mode 100644 index 0000000..57af7c3 --- /dev/null +++ b/app/Exceptions/InstallStateUnknownException.php @@ -0,0 +1,12 @@ + 'Enter the tags to add.', ]); - $items = Item::query()->whereIn('id', $data['ids'])->get(); $tags = Tag::parseList($data['tags'] ?? null); + $count = 0; - foreach ($items as $item) { - switch ($data['action']) { - case 'archive': - $item->archive(); - break; - case 'unarchive': - $item->unarchive(); - break; - case 'favourite': - $item->setFavourite(true); - break; - case 'unfavourite': - $item->setFavourite(false); - break; - case 'tag': - $item->attachTagNames($tags); - break; - case 'retry': - $this->retry($item); - break; - case 'delete': - // One by one, so each item's stored images go too. - $item->delete(); - break; - } - } + // In batches, without the article bodies: a thousand ticked items + // must not mean a thousand full articles in memory at once. + Item::query() + ->withoutBodies() + ->whereIn('items.id', $data['ids']) + ->chunkById(100, function ($items) use ($data, $tags, &$count): void { + foreach ($items as $item) { + $count++; - $count = $items->count(); + switch ($data['action']) { + case 'archive': + $item->archive(); + break; + case 'unarchive': + $item->unarchive(); + break; + case 'favourite': + $item->setFavourite(true); + break; + case 'unfavourite': + $item->setFavourite(false); + break; + case 'tag': + $item->attachTagNames($tags); + break; + case 'retry': + $this->retry($item); + break; + case 'delete': + // One by one, so each item's stored images go too. + $item->delete(); + break; + } + } + }, 'items.id', 'id'); return back()->with('status', sprintf( '%s: %d %s.', diff --git a/app/Http/Controllers/ExportController.php b/app/Http/Controllers/ExportController.php index e961014..8dfc703 100644 --- a/app/Http/Controllers/ExportController.php +++ b/app/Http/Controllers/ExportController.php @@ -3,25 +3,29 @@ namespace App\Http\Controllers; use App\Services\Exporter; +use Illuminate\Http\Request; use Symfony\Component\HttpFoundation\StreamedResponse; class ExportController extends Controller { /** - * Pocket-shaped CSV: title, url, time_added, tags, status. + * Pocket-shaped CSV: title, url, time_added, tags, status. ?for=spreadsheet + * neutralises cells a spreadsheet would run as formulas. */ - public function csv(Exporter $exporter): StreamedResponse + public function csv(Request $request, Exporter $exporter): StreamedResponse { + $forSpreadsheets = $request->query('for') === 'spreadsheet'; + return response()->streamDownload( - function () use ($exporter): void { + function () use ($exporter, $forSpreadsheets): void { $handle = fopen('php://output', 'w'); if ($handle !== false) { - $exporter->writeCsv($handle); + $exporter->writeCsv($handle, $forSpreadsheets); fclose($handle); } }, - 'docket-export-'.now()->format('Y-m-d').'.csv', + 'docket-export-'.now()->format('Y-m-d').($forSpreadsheets ? '-spreadsheet' : '').'.csv', ['Content-Type' => 'text/csv; charset=UTF-8'] ); } diff --git a/app/Http/Controllers/InstallController.php b/app/Http/Controllers/InstallController.php index a361b56..409b037 100644 --- a/app/Http/Controllers/InstallController.php +++ b/app/Http/Controllers/InstallController.php @@ -36,17 +36,16 @@ public function requirements(): View /** * Step 2: the setup form (site URL + owner account). */ - public function form(Request $request): View|RedirectResponse + public function form(Request $request): Response|RedirectResponse { if ($this->installer->blockingProblems() !== []) { return redirect()->route('install.requirements'); } - return view('install.setup', [ - 'input' => ['url' => $request->getSchemeAndHttpHost()], - 'errors' => new MessageBag(), - 'installError' => null, - ]); + // Creates the code on first visit, so it is there to read. + $this->installer->setupCode(); + + return $this->setupView(['url' => $request->getSchemeAndHttpHost()], new MessageBag(), null); } /** @@ -59,36 +58,43 @@ public function store(Request $request): View|RedirectResponse|Response } $validator = Validator::make($request->all(), [ + 'setup_code' => ['required', 'string', 'max:64'], 'url' => ['required', 'string', 'url', 'max:255'], 'name' => ['required', 'string', 'max:255'], 'email' => ['required', 'string', 'email', 'max:255'], 'password' => ['required', 'string', 'min:8', 'confirmed'], ]); + $validator->after(function ($validator) use ($request): void { + if (is_string($request->input('setup_code')) && ! $this->installer->checkSetupCode($request->input('setup_code'))) { + $validator->errors()->add('setup_code', 'That setup code is not right. Copy it from the file named below.'); + } + }); + $safeInput = $request->except(['password', 'password_confirmation']); if ($validator->fails()) { - return response()->view('install.setup', [ - 'input' => $safeInput, - 'errors' => $validator->errors(), - 'installError' => null, - ], 422); + return $this->setupView($safeInput, $validator->errors(), null, 422); } $data = $validator->validated(); + // One installation at a time, and the "already installed?" question + // asked again inside the lock: two requests that both got past the + // middleware can't both create an owner. + $lock = $this->installer->acquireLock(); + + if ($lock === null) { + return $this->setupView($safeInput, new MessageBag(), 'Another installation is running right now. Wait a moment, then reload.', 409); + } + try { + if ($this->installer->isInstalled()) { + return redirect()->route('login'); + } + $this->installer->ensureEnvFile(); $this->installer->ensureAppKey(); - - // The password is deliberately NOT written to .env: it is - // hashed straight into the database below. - $this->installer->writeEnvValues([ - 'APP_URL' => $data['url'], - 'DOCKET_USER_NAME' => $data['name'], - 'DOCKET_USER_EMAIL' => $data['email'], - ]); - $this->installer->ensureSqliteDatabase(); if (! $this->installer->migrate()) { @@ -96,14 +102,24 @@ public function store(Request $request): View|RedirectResponse|Response } $this->installer->createOwner($data['name'], $data['email'], $data['password']); + + // Configuration changes only once the account exists, so a + // failed or refused install leaves .env as it was. The password + // is deliberately NOT written to .env: it is hashed into the + // database above. + $this->installer->writeEnvValues([ + 'APP_URL' => $data['url'], + 'DOCKET_USER_NAME' => $data['name'], + 'DOCKET_USER_EMAIL' => $data['email'], + ]); + + $this->installer->markInstalled(); } catch (Throwable $e) { report($e); - return response()->view('install.setup', [ - 'input' => $safeInput, - 'errors' => new MessageBag(), - 'installError' => $e->getMessage(), - ], 500); + return $this->setupView($safeInput, new MessageBag(), $e->getMessage(), 500); + } finally { + $this->installer->releaseLock($lock); } return view('install.complete', [ @@ -112,4 +128,17 @@ public function store(Request $request): View|RedirectResponse|Response 'assetsBuilt' => file_exists(public_path('build/manifest.json')), ]); } + + /** + * @param array $input + */ + protected function setupView(array $input, MessageBag $errors, ?string $installError, int $status = 200): Response + { + return response()->view('install.setup', [ + 'input' => $input, + 'errors' => $errors, + 'installError' => $installError, + 'codeLocation' => $this->installer->setupCodeLocation(), + ], $status); + } } diff --git a/app/Http/Controllers/ItemController.php b/app/Http/Controllers/ItemController.php index 5af9f95..8277b00 100644 --- a/app/Http/Controllers/ItemController.php +++ b/app/Http/Controllers/ItemController.php @@ -51,7 +51,7 @@ protected function list(Request $request, string $view, ?Tag $tag = null): View $sort = $request->query('sort'); $sort = is_string($sort) && array_key_exists($sort, Item::SORTS) ? $sort : null; - $query = Item::query()->with('tags'); + $query = Item::query()->withoutBodies()->with('tags'); if ($view === 'archive') { $query->archived(); diff --git a/app/Http/Middleware/EnsureNotInstalled.php b/app/Http/Middleware/EnsureNotInstalled.php index b6620ad..78cbda5 100644 --- a/app/Http/Middleware/EnsureNotInstalled.php +++ b/app/Http/Middleware/EnsureNotInstalled.php @@ -2,6 +2,7 @@ namespace App\Http\Middleware; +use App\Exceptions\InstallStateUnknownException; use App\Services\Installer; use Closure; use Illuminate\Http\Request; @@ -10,7 +11,9 @@ /** * Gates the web installer: once Docket has its owner account the /install * routes stop existing in any useful sense and just bounce to the login - * screen, so a live site never exposes a working installer. + * screen, so a live site never exposes a working installer. If the + * database can't be read, the installer stays closed too (503) rather + * than mistaking a broken site for a fresh one. */ class EnsureNotInstalled { @@ -23,7 +26,15 @@ public function __construct(protected Installer $installer) {} */ public function handle(Request $request, Closure $next): Response { - if ($this->installer->isInstalled()) { + try { + $installed = $this->installer->isInstalled(); + } catch (InstallStateUnknownException $e) { + report($e); + + return response()->view('install.unavailable', ['message' => $e->getMessage()], 503); + } + + if ($installed) { return redirect()->route('login'); } diff --git a/app/Jobs/FetchItemContent.php b/app/Jobs/FetchItemContent.php index 0005c9c..1be177a 100644 --- a/app/Jobs/FetchItemContent.php +++ b/app/Jobs/FetchItemContent.php @@ -13,6 +13,7 @@ use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Queue\Queueable; use Illuminate\Queue\Jobs\SyncJob; +use Illuminate\Queue\Middleware\WithoutOverlapping; use Illuminate\Support\Str; use Throwable; @@ -32,6 +33,8 @@ class FetchItemContent implements ShouldQueue /** * Image downloads can add up; this is the ceiling for the whole job. + * The queue's retry_after (config/queue.php) must stay above it, or a + * second worker picks the job up while the first is still running. */ public int $timeout = 300; @@ -42,6 +45,22 @@ class FetchItemContent implements ShouldQueue public function __construct(public Item $item) {} + /** + * One fetch per item at a time: a retry or refetch queued while an + * earlier fetch of the same item is still running is dropped, not run + * alongside it (the running one is about to store the article anyway). + * + * @return list + */ + public function middleware(): array + { + return [ + (new WithoutOverlapping('item-'.$this->item->getKey())) + ->dontRelease() + ->expireAfter($this->timeout + 60), + ]; + } + public function tries(): int { return max(1, (int) settings('fetch.tries')); @@ -92,27 +111,45 @@ public function handle( $html = $article->html !== null ? $sanitiser->sanitise($article->html) : ''; - $item->forceFill([ + $metadata = [ 'resolved_url' => $response->url !== $item->url ? $response->url : null, 'author' => $article->author, 'site_name' => $article->siteName, 'lead_image_url' => $article->leadImageUrl, 'language' => $article->language, 'published_at' => $article->publishedAt, - ]); - - if ($this->shouldReplaceTitle($item) && $article->title !== null) { - $item->title = $article->title; - } + 'excerpt' => $article->excerpt, + ]; if (trim(strip_tags($html, '')) === '') { - $item->forceFill(['excerpt' => $article->excerpt])->save(); + // Nothing readable this time. Fill in only what was missing, so + // a refresh of a page that has since changed or broken doesn't + // wipe the details of the copy already saved. + foreach ($metadata as $field => $value) { + if ($value !== null && blank($item->getAttribute($field))) { + $item->setAttribute($field, $value); + } + } + + if ($this->shouldReplaceTitle($item) && $article->title !== null) { + $item->title = $article->title; + } + + $item->save(); $this->recordFailure('Could not find any article text on this page. The original link still works.'); return; } - $html = $images->localise($item, $html); + $item->forceFill($metadata); + + if ($this->shouldReplaceTitle($item) && $article->title !== null) { + $item->title = $article->title; + } + + // One budget for the inline images and the lead image. + $downloads = $images->forItem($item); + $html = $downloads->localise($html); $text = HtmlSanitiser::toText($html); $words = HtmlSanitiser::wordCount($text); $wordsPerMinute = max(1, (int) settings('words_per_minute')); @@ -123,9 +160,7 @@ public function handle( 'excerpt' => $article->excerpt ?? Str::limit(str_replace("\n", ' ', $text), 280), 'word_count' => $words, 'reading_time' => $words > 0 ? (int) max(1, ceil($words / $wordsPerMinute)) : 0, - 'lead_image_path' => $article->leadImageUrl !== null - ? $images->store($item, $article->leadImageUrl) - : null, + 'lead_image_path' => $downloads->lead($article->leadImageUrl), 'fetch_status' => FetchStatus::Fetched, 'fetch_error' => null, 'fetched_at' => now(), diff --git a/app/Models/Item.php b/app/Models/Item.php index e64d53a..06fd0e8 100644 --- a/app/Models/Item.php +++ b/app/Models/Item.php @@ -70,6 +70,20 @@ class Item extends Model 'longest' => 'Longest', ]; + /** + * Every column except the two article bodies, which can run to + * hundreds of kilobytes each: what lists, bulk actions and the CSV + * export select. + */ + public const LIST_COLUMNS = [ + 'id', 'url', 'normalized_url', 'resolved_url', 'title', 'author', + 'site_name', 'excerpt', 'lead_image_url', 'lead_image_path', + 'word_count', 'reading_time', 'language', 'published_at', 'status', + 'archived_at', 'is_favourite', 'favourited_at', 'read_progress', + 'last_read_at', 'source', 'fetch_status', 'fetch_error', + 'fetch_attempts', 'fetched_at', 'added_at', 'created_at', 'updated_at', + ]; + protected $fillable = [ 'url', 'normalized_url', @@ -194,9 +208,21 @@ public function isArchived(): bool return $this->status === ItemStatus::Archived; } + /** + * Whether a readable copy is stored. Independent of the latest fetch: + * a failed refresh keeps the last good copy readable. + */ public function hasArticle(): bool { - return $this->fetch_status === FetchStatus::Fetched && trim((string) $this->content_html) !== ''; + return trim((string) $this->content_html) !== ''; + } + + /** + * A copy is stored but the latest attempt to refresh it failed. + */ + public function refreshFailed(): bool + { + return $this->fetch_status === FetchStatus::Failed && $this->hasArticle(); } public function archive(): void @@ -248,6 +274,16 @@ public function syncTagNames(iterable $names): void $this->tags()->sync(Tag::idsForNames($names)); } + /** + * Select everything but the article bodies. + * + * @param Builder $query + */ + public function scopeWithoutBodies(Builder $query): void + { + $query->select(array_map(fn (string $column): string => 'items.'.$column, self::LIST_COLUMNS)); + } + /** * @param Builder $query */ @@ -300,8 +336,13 @@ public function scopeSearch(Builder $query, string $term, bool $orderByRelevance ->selectRaw('rowid AS fts_rowid, bm25(items_fts, 10.0, 5.0, 1.0) AS fts_rank') ->whereRaw('items_fts MATCH ?', [$match]); - $query->joinSub($matches, 'fts', 'fts.fts_rowid', '=', 'items.id') - ->select('items.*'); + // Keep any column list the caller chose (withoutBodies()); otherwise + // select the item's own columns, not the join's. + if ($query->getQuery()->columns === null) { + $query->select('items.*'); + } + + $query->joinSub($matches, 'fts', 'fts.fts_rowid', '=', 'items.id'); if ($orderByRelevance) { $query->orderBy('fts.fts_rank'); diff --git a/app/Models/Tag.php b/app/Models/Tag.php index f879b19..fc18a15 100644 --- a/app/Models/Tag.php +++ b/app/Models/Tag.php @@ -24,11 +24,6 @@ class Tag extends Model protected $fillable = ['name', 'slug']; - public function getRouteKeyName(): string - { - return 'slug'; - } - /** * @return BelongsToMany */ @@ -50,7 +45,9 @@ public static function cleanName(string $name): string } /** - * The case-insensitive identity of a tag name. Not Str::slug(): that + * The case-insensitive identity of a tag name, used to match names, not + * in URLs: a name may contain "/" or other characters a route can't + * carry, so tags are routed by ID. Not Str::slug(): that * transliterates to ASCII and would turn a non-Latin tag into nothing. */ public static function slugFor(string $name): string diff --git a/app/Services/Exporter.php b/app/Services/Exporter.php index c4e383a..0ff733e 100644 --- a/app/Services/Exporter.php +++ b/app/Services/Exporter.php @@ -21,26 +21,43 @@ class Exporter public const CSV_COLUMNS = ['title', 'url', 'time_added', 'tags', 'status']; /** + * Pocket's CSV. Lossless by default, so it imports back exactly. With + * $forSpreadsheets, any cell a spreadsheet would run as a formula gets + * a leading apostrophe: titles and tags come from other people's web + * pages, and "=HYPERLINK(...)" in a title is someone else's formula. + * * @param resource $handle */ - public function writeCsv($handle): void + public function writeCsv($handle, bool $forSpreadsheets = false): void { fputcsv($handle, self::CSV_COLUMNS, ',', '"', ''); - foreach ($this->items() as $item) { - fputcsv($handle, [ + foreach ($this->items(withBodies: false) as $item) { + $row = [ $item->title ?? '', $item->url, (string) ($item->added_at?->getTimestamp() ?? ''), $item->tags->pluck('name')->implode('|'), $item->status->pocketName(), - ], ',', '"', ''); + ]; + + fputcsv($handle, $forSpreadsheets ? array_map(self::spreadsheetSafe(...), $row) : $row, ',', '"', ''); } } /** - * Streamed one item at a time so a large library never has to fit in - * memory as a single array. + * Neutralise a cell a spreadsheet would treat as a formula: one starting + * with = + - or @, including after leading whitespace or control + * characters, or starting with a tab or carriage return. + */ + public static function spreadsheetSafe(string $value): string + { + return preg_match('/^[\s\x00-\x1f]*[=+\-@]|^[\t\r]/u', $value) === 1 ? "'".$value : $value; + } + + /** + * Written one item at a time, from small batches, so a large library + * never has to fit in memory. * * @param resource $handle */ @@ -52,7 +69,7 @@ public function writeJson($handle): void $first = true; - foreach ($this->items() as $item) { + foreach ($this->items(withBodies: true) as $item) { fwrite($handle, ($first ? "\n" : ",\n").json_encode($this->itemToArray($item), $flags)); $first = false; } @@ -99,16 +116,17 @@ public function itemToArray(Item $item): array } /** - * Oldest first, so an export re-imported in order keeps its sequence. + * Every item in ID order, fetched in small keyset batches (stable even + * if items change mid-export). The CSV needs no article bodies; the + * JSON does, so its batches are smaller. * * @return LazyCollection */ - protected function items(): LazyCollection + protected function items(bool $withBodies): LazyCollection { return Item::query() + ->when(! $withBodies, fn ($query) => $query->withoutBodies()) ->with('tags') - ->orderBy('added_at') - ->orderBy('id') - ->lazy(200); + ->lazyById($withBodies ? 25 : 500, 'items.id', 'id'); } } diff --git a/app/Services/Fetching/IpGuard.php b/app/Services/Fetching/IpGuard.php index eace071..ef4329a 100644 --- a/app/Services/Fetching/IpGuard.php +++ b/app/Services/Fetching/IpGuard.php @@ -50,6 +50,16 @@ class IpGuard 'ff00::/8', // multicast ]; + /** + * The check SafeHttpClient runs on every resolved address. An instance + * method (bound in the container) so the real-transport integration + * test can point a hostname at a local fixture server. + */ + public function allows(string $ip): bool + { + return self::isPublic($ip); + } + public static function isPublic(string $ip): bool { $ip = trim($ip, '[] '); diff --git a/app/Services/Fetching/SafeHttpClient.php b/app/Services/Fetching/SafeHttpClient.php index 6cc0231..d1600ce 100644 --- a/app/Services/Fetching/SafeHttpClient.php +++ b/app/Services/Fetching/SafeHttpClient.php @@ -3,7 +3,10 @@ namespace App\Services\Fetching; use App\Exceptions\FetchFailedException; +use App\Exceptions\ResponseTooLargeException; use App\Exceptions\UnsafeUrlException; +use GuzzleHttp\Exception\TransferException; +use GuzzleHttp\Handler\CurlHandler; use GuzzleHttp\Psr7\UriResolver; use GuzzleHttp\Psr7\Utils; use Illuminate\Http\Client\ConnectionException; @@ -23,7 +26,14 @@ * 127.0.0.1 between the check and the connect (DNS rebinding); * - redirects are followed by hand, and each hop is vetted the same way; * - connect and total timeouts, a redirect cap, and a byte cap enforced - * both on Content-Length and while streaming the body. + * on Content-Length and while the body downloads (a progress callback + * aborts the transfer as soon as it passes the cap). + * + * The transport is always Guzzle's cURL handler. Address pinning is a + * cURL option, and Guzzle's default handler stack sends some requests + * through its PHP-stream handler instead, which refuses (Guzzle 8) or + * silently ignores (Guzzle 7) cURL options. Http::fake() still works: the + * fake sits in front of whichever handler is set. * * If the host routes outbound traffic through an HTTP proxy (HTTPS_PROXY * and friends), the proxy does its own DNS lookup and the pinning above @@ -31,24 +41,28 @@ */ class SafeHttpClient { - public function __construct(protected HostResolver $resolver) {} + public function __construct( + protected HostResolver $resolver, + protected IpGuard $guard, + ) {} /** * GET a URL and return its body, refusing anything unsafe. * * @param int $maxBytes abandon the download past this many bytes * @param string $accept the Accept header to send + * @param int|null $timeout seconds per request; the fetch.timeout setting when null * * @throws FetchFailedException */ - public function get(string $url, int $maxBytes, string $accept = '*/*'): FetchedResponse + public function get(string $url, int $maxBytes, string $accept = '*/*', ?int $timeout = null): FetchedResponse { $maxRedirects = (int) config('docket.fetch.max_redirects', 5); for ($hop = 0; ; $hop++) { [$host, $port, $address] = $this->vet($url); - $response = $this->send($url, $host, $port, $address, $accept); + $response = $this->send($url, $host, $port, $address, $accept, $maxBytes, $timeout ?? (int) settings('fetch.timeout')); $status = $response->status(); $location = $response->header('Location'); @@ -73,7 +87,7 @@ public function get(string $url, int $maxBytes, string $accept = '*/*'): Fetched $length = $response->header('Content-Length'); if (is_numeric($length) && (int) $length > $maxBytes) { - throw FetchFailedException::permanent($this->tooLargeMessage($maxBytes)); + throw new ResponseTooLargeException($this->tooLargeMessage($maxBytes), $maxBytes); } return new FetchedResponse( @@ -81,7 +95,7 @@ public function get(string $url, int $maxBytes, string $accept = '*/*'): Fetched status: $status, contentType: $contentType, charset: $charset, - body: $this->readBody($response, $maxBytes), + body: $this->body($response, $maxBytes), ); } } @@ -112,7 +126,7 @@ public function vet(string $url): array } foreach ($addresses as $address) { - if (! IpGuard::isPublic($address)) { + if (! $this->guard->allows($address)) { throw new UnsafeUrlException("Refused to fetch {$host}: it points at a private or local network address."); } } @@ -123,7 +137,7 @@ public function vet(string $url): array /** * @throws FetchFailedException */ - protected function send(string $url, string $host, int $port, string $address, string $accept): Response + protected function send(string $url, string $host, int $port, string $address, string $accept, int $maxBytes, int $timeout): Response { $pinned = str_contains($address, ':') ? "[{$address}]" : $address; @@ -132,46 +146,50 @@ protected function send(string $url, string $host, int $port, string $address, s ? [CURLOPT_RESOLVE => ["{$host}:{$port}:{$pinned}"]] : []; + $tooLarge = false; + try { return Http::withHeaders([ 'User-Agent' => (string) config('docket.fetch.user_agent'), 'Accept' => $accept, 'Accept-Language' => 'en-GB,en;q=0.9,*;q=0.5', ]) - ->timeout((int) settings('fetch.timeout')) + ->setHandler(new CurlHandler()) + ->timeout(max(1, $timeout)) ->connectTimeout((int) config('docket.fetch.connect_timeout', 10)) ->withoutRedirecting() ->withOptions([ - 'stream' => true, 'protocols' => ['http', 'https'], 'curl' => $curl, + // Returning true aborts the transfer. The expected size + // is known as soon as the headers arrive, so an + // oversized Content-Length stops before the body. + 'progress' => function (int $expected, int $downloaded) use ($maxBytes, &$tooLarge): bool { + return $tooLarge = $expected > $maxBytes || $downloaded > $maxBytes; + }, ]) ->get($url); - } catch (ConnectionException $e) { + } catch (ConnectionException|TransferException $e) { + if ($tooLarge) { + throw new ResponseTooLargeException($this->tooLargeMessage($maxBytes), $maxBytes); + } + throw new FetchFailedException('Could not connect: '.$this->shortError($e->getMessage())); } } /** - * Read the body in chunks, giving up as soon as it passes the cap. + * The body, with the cap checked again for transports that don't report + * progress (Http::fake() in tests). * * @throws FetchFailedException */ - protected function readBody(Response $response, int $maxBytes): string + protected function body(Response $response, int $maxBytes): string { - $stream = $response->toPsrResponse()->getBody(); - $body = ''; + $body = $response->body(); - try { - while (! $stream->eof()) { - $body .= $stream->read(65536); - - if (strlen($body) > $maxBytes) { - throw FetchFailedException::permanent($this->tooLargeMessage($maxBytes)); - } - } - } finally { - $stream->close(); + if (strlen($body) > $maxBytes) { + throw new ResponseTooLargeException($this->tooLargeMessage($maxBytes), $maxBytes); } return $body; diff --git a/app/Services/ImageDownloads.php b/app/Services/ImageDownloads.php new file mode 100644 index 0000000..628db0e --- /dev/null +++ b/app/Services/ImageDownloads.php @@ -0,0 +1,91 @@ + stored filename, or null when it wasn't stored. + * + * @var array + */ + protected array $results = []; + + public function __construct( + protected ImageLocaliser $localiser, + protected Item $item, + ) { + $this->attemptsLeft = config('docket.images.enabled') ? max(0, (int) settings('images.max_per_item')) : 0; + $this->bytesLeft = max(0, (int) config('docket.images.max_total_kb')) * 1024; + $this->deadline = microtime(true) + max(0, (int) config('docket.images.time_budget')); + } + + public function localise(string $html): string + { + return $this->localiser->rewrite($this->item, $html, fn (string $url): ?string => $this->download($url)); + } + + /** + * The stored filename for the lead image, if it could be stored. + */ + public function lead(?string $url): ?string + { + return $url === null ? null : $this->download($url); + } + + /** + * How many download attempts are left (for tests and logging). + */ + public function attemptsLeft(): int + { + return $this->attemptsLeft; + } + + protected function download(string $url): ?string + { + if (array_key_exists($url, $this->results)) { + return $this->results[$url]; + } + + $secondsLeft = (int) floor($this->deadline - microtime(true)); + + if ($this->attemptsLeft <= 0 || $this->bytesLeft <= 0 || $secondsLeft < 1) { + return $this->results[$url] = null; + } + + $this->attemptsLeft--; + + [$filename, $bytes] = $this->localiser->store( + $this->item, + $url, + min((int) config('docket.images.max_kb') * 1024, $this->bytesLeft), + min($secondsLeft, (int) settings('fetch.timeout')), + ); + + $this->bytesLeft -= $bytes; + + return $this->results[$url] = $filename; + } +} diff --git a/app/Services/ImageLocaliser.php b/app/Services/ImageLocaliser.php index 39cce59..5ffb491 100644 --- a/app/Services/ImageLocaliser.php +++ b/app/Services/ImageLocaliser.php @@ -3,6 +3,7 @@ namespace App\Services; use App\Exceptions\FetchFailedException; +use App\Exceptions\ResponseTooLargeException; use App\Models\Item; use App\Services\Fetching\SafeHttpClient; use DOMElement; @@ -10,6 +11,7 @@ use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use Masterminds\HTML5; +use Throwable; /** * Downloads an article's images to private storage and points their src @@ -18,8 +20,9 @@ * * Images go through SafeHttpClient like any other fetch. Only raster * formats are kept (SVG can carry scripts and would be served from this - * origin); anything that fails, is too big, or is past the per-article cap - * keeps its remote src. + * origin); anything that fails, is too big, or is past the article's + * budget keeps its remote src. The budget (ImageDownloads) is shared by the + * inline images and the lead image of one article. */ class ImageLocaliser { @@ -43,13 +46,32 @@ class ImageLocaliser public function __construct(protected SafeHttpClient $http) {} /** - * Rewrite every in already-sanitised article HTML. + * A download budget for one article: use it for the inline images and + * then the lead image, so both share one set of limits and the lead + * image reuses an inline copy. + */ + public function forItem(Item $item): ImageDownloads + { + return new ImageDownloads($this, $item); + } + + /** + * Rewrite every in already-sanitised article HTML, with a fresh + * budget. Use forItem() when the lead image should share it. */ public function localise(Item $item, string $html): string { - $limit = (int) settings('images.max_per_item'); + return $this->forItem($item)->localise($html); + } - if (! config('docket.images.enabled') || $limit < 1 || ! str_contains($html, ' src that $download stores to the local copy. + * + * @param callable(string): ?string $download URL in, stored filename (or null) out + */ + public function rewrite(Item $item, string $html, callable $download): string + { + if (! str_contains($html, 'getElementsByTagName('img')) as $img) { $src = $img->getAttribute('src'); @@ -73,14 +93,10 @@ public function localise(Item $item, string $html): string continue; } - if (! array_key_exists($src, $stored)) { - $stored[$src] = count(array_filter($stored)) < $limit - ? $this->store($item, $src) - : null; - } + $filename = $download($src); - if ($stored[$src] !== null) { - $img->setAttribute('src', $this->url($item, $stored[$src])); + if ($filename !== null) { + $img->setAttribute('src', $this->url($item, $filename)); } } @@ -90,32 +106,50 @@ public function localise(Item $item, string $html): string /** * Download one image into the item's directory. * - * @return string|null the stored filename, or null if it was not stored + * @return array{0: string|null, 1: int} the stored filename (null if + * not stored) and bytes downloaded */ - public function store(Item $item, string $url): ?string + public function store(Item $item, string $url, int $maxBytes, int $timeout): array { try { - $response = $this->http->get($url, (int) config('docket.images.max_kb') * 1024, 'image/avif,image/webp,image/png,image/jpeg,image/gif;q=0.9,*/*;q=0.1'); + $response = $this->http->get($url, $maxBytes, 'image/avif,image/webp,image/png,image/jpeg,image/gif;q=0.9,*/*;q=0.1', $timeout); } catch (FetchFailedException $e) { Log::debug('Image not stored', ['item' => $item->id, 'url' => $url, 'reason' => $e->getMessage()]); - return null; + // An abandoned oversized download spent (up to) its cap. + return [null, $e instanceof ResponseTooLargeException ? $e->limit : 0]; } + $bytes = strlen($response->body); + // Trust the bytes, not the header: a server can label anything // image/png. $sniffed = (new \finfo(FILEINFO_MIME_TYPE))->buffer($response->body); $extension = self::TYPES[$sniffed] ?? null; if ($extension === null) { - return null; + return [null, $bytes]; } $filename = sha1($url).'.'.$extension; + $path = $item->storageDirectory().'/'.$filename; - Storage::disk(Item::DISK)->put($item->storageDirectory().'/'.$filename, $response->body); + // A failed write (full disk, permissions) must leave the working + // remote src in place, not point the article at a missing file. + try { + $written = Storage::disk(Item::DISK)->put($path, $response->body); + } catch (Throwable $e) { + report($e); + $written = false; + } + + if ($written === false) { + Log::warning('Could not store an article image; keeping its remote address.', ['item' => $item->id, 'path' => $path]); + + return [null, $bytes]; + } - return $filename; + return [$filename, $bytes]; } /** diff --git a/app/Services/Installer.php b/app/Services/Installer.php index 966ba64..828491d 100644 --- a/app/Services/Installer.php +++ b/app/Services/Installer.php @@ -2,11 +2,14 @@ namespace App\Services; +use App\Exceptions\InstallStateUnknownException; use App\Models\User; use Illuminate\Contracts\Foundation\Application; use Illuminate\Support\Facades\Artisan; use Illuminate\Support\Facades\Schema; +use Illuminate\Support\Str; use PDO; +use RuntimeException; use Throwable; /** @@ -14,24 +17,128 @@ * (php artisan docket:install) and the web installer (/install). * Each step is a small idempotent method so either front-end can run * them one at a time and narrate progress in its own way. + * + * Docket has exactly one account, and the web installer is reachable by + * anyone who finds a fresh deployment, so installation is guarded three + * ways: the web form needs a setup code that can only be read on the + * server; installations are serialised by a lock and re-check ownership + * inside it; and createOwner() refuses outright once any account exists. + * Once installed, a marker file keeps the installer closed even when the + * database can't be read. */ class Installer { public function __construct(protected Application $app) {} /** - * Docket counts as installed once the owner account exists — the - * final step of either installer. Anything earlier (missing .env, - * empty database) is a fresh or half-finished install. + * Docket counts as installed once the installed marker exists or an + * account does. A missing SQLite file is a fresh install; any other + * database error is not proof of anything, so it fails closed rather + * than reopening the installer on a live site. + * + * @throws InstallStateUnknownException */ public function isInstalled(): bool { + if (is_file($this->statePath('installed'))) { + return true; + } + try { - return Schema::hasTable('users') && User::query()->exists(); - } catch (Throwable) { - // No database yet — definitely a fresh install. - return false; + $installed = Schema::hasTable('users') && User::query()->exists(); + } catch (Throwable $e) { + if ($this->databaseIsMissing()) { + return false; + } + + throw new InstallStateUnknownException( + 'Docket could not read its database to check whether it is installed, so the installer stays closed. Check storage/logs/laravel.log.', + previous: $e, + ); + } + + if ($installed) { + // Installs from before the marker existed get one now. + $this->markInstalled(); + } + + return $installed; + } + + /** + * Record a finished installation and retire the setup code. + */ + public function markInstalled(): void + { + $this->ensureStateDirectory(); + @file_put_contents($this->statePath('installed'), now()->toIso8601String()."\n"); + @unlink($this->statePath('setup-code')); + } + + /** + * The one-time code the web installer asks for, created on first use. + * Reading it needs shell access to the server, which is what proves + * the person at /install is the person deploying Docket. + */ + public function setupCode(): string + { + $path = $this->statePath('setup-code'); + + if (! is_file($path)) { + $this->ensureStateDirectory(); + file_put_contents($path, implode('-', str_split(Str::lower(Str::random(16)), 4))."\n"); + @chmod($path, 0600); + } + + return trim((string) file_get_contents($path)); + } + + public function checkSetupCode(string $given): bool + { + return hash_equals($this->setupCode(), strtolower(trim($given))); + } + + /** + * Where the setup code lives, relative to the app root when it is + * inside it, for showing in the installer. + */ + public function setupCodeLocation(): string + { + return Str::after($this->statePath('setup-code'), $this->app->basePath().DIRECTORY_SEPARATOR); + } + + /** + * Take the installation lock without waiting: a second installation + * running at the same moment is told to try again rather than queued + * behind the first. + * + * @return resource|null the lock handle, or null when it is held + */ + public function acquireLock() + { + $this->ensureStateDirectory(); + $handle = fopen($this->statePath('install.lock'), 'c'); + + if ($handle === false) { + throw new RuntimeException('Could not open the installation lock file.'); + } + + if (! flock($handle, LOCK_EX | LOCK_NB)) { + fclose($handle); + + return null; } + + return $handle; + } + + /** + * @param resource $handle + */ + public function releaseLock($handle): void + { + flock($handle, LOCK_UN); + fclose($handle); } /** @@ -56,7 +163,7 @@ public function requirements(): array ], ]; - foreach (['pdo_sqlite', 'mbstring', 'fileinfo', 'dom', 'xml'] as $extension) { + foreach (['pdo_sqlite', 'mbstring', 'fileinfo', 'dom', 'xml', 'curl'] as $extension) { $checks[] = [ 'label' => "PHP extension: {$extension}", 'ok' => extension_loaded($extension), @@ -144,7 +251,7 @@ public function ensureEnvFile(): bool $example = $this->app->environmentPath().DIRECTORY_SEPARATOR.'.env.example'; if (! copy($example, $envPath)) { - throw new \RuntimeException("Could not create {$envPath} from .env.example."); + throw new RuntimeException("Could not create {$envPath} from .env.example."); } return true; @@ -230,8 +337,23 @@ public function ownerExists(string $email): bool return User::query()->where('email', $email)->exists(); } + /** + * Whether any account exists. Docket has one, whatever its email. + */ + public function hasOwner(): bool + { + return User::query()->exists(); + } + + /** + * @throws RuntimeException when an account already exists + */ public function createOwner(string $name, string $email, string $plainPassword): User { + if ($this->hasOwner()) { + throw new RuntimeException('Docket already has its owner account; it does not create a second one.'); + } + // The User model's 'hashed' cast hashes the plain password. return User::query()->create([ 'name' => $name, @@ -240,6 +362,31 @@ public function createOwner(string $name, string $email, string $plainPassword): ]); } + protected function statePath(string $file): string + { + return rtrim((string) config('docket.install.state_path'), DIRECTORY_SEPARATOR).DIRECTORY_SEPARATOR.$file; + } + + protected function ensureStateDirectory(): void + { + $directory = dirname($this->statePath('x')); + + if (! is_dir($directory)) { + mkdir($directory, 0700, true); + } + } + + /** + * True only for a fresh install: SQLite whose database file doesn't + * exist yet (or is still empty). + */ + protected function databaseIsMissing(): bool + { + $path = $this->sqlitePath(); + + return $path !== null && (! is_file($path) || filesize($path) === 0); + } + /** * Whether this PHP's SQLite library was compiled with FTS5, which the * items_fts search index needs. diff --git a/app/Services/Settings.php b/app/Services/Settings.php index 2dbc208..40bc8f9 100644 --- a/app/Services/Settings.php +++ b/app/Services/Settings.php @@ -76,7 +76,7 @@ public static function fields(): array ], 'images.max_per_item' => [ 'label' => 'Images saved per article', - 'help' => 'Inline images downloaded so articles survive link rot. Set to 0 to keep every image remote.', + 'help' => 'Images downloaded (or tried) per article, lead image included, so articles survive link rot. Set to 0 to keep every image remote.', 'type' => 'int', 'rules' => ['required', 'integer', 'min:0', 'max:500'], ], diff --git a/config/docket.php b/config/docket.php index b67ffae..a2cb84a 100644 --- a/config/docket.php +++ b/config/docket.php @@ -17,17 +17,27 @@ /* | The single owner account. There is no registration flow: the installer - | (php artisan docket:install or /install) creates the account directly, - | and these are the fallbacks DatabaseSeeder uses if the database is ever - | re-seeded from scratch. Once the user exists the database is the source - | of truth, so changing these afterwards has no effect on the live login. + | (php artisan docket:install or /install) creates the account directly. + | DatabaseSeeder uses these only on an empty users table, and refuses to + | run without DOCKET_USER_PASSWORD: there is deliberately no default + | password. Once the user exists the database is the source of truth. */ 'user' => [ // ?: not the env() default param: a present-but-empty line in .env // (e.g. DOCKET_USER_NAME=) would otherwise seed an empty string. 'name' => env('DOCKET_USER_NAME') ?: 'Docket Owner', 'email' => env('DOCKET_USER_EMAIL') ?: 'owner@example.com', - 'password' => env('DOCKET_USER_PASSWORD') ?: 'change-me-on-first-login', + 'password' => env('DOCKET_USER_PASSWORD') ?: null, + ], + + /* + | Installer state: the "installed" marker that keeps /install closed even + | if the database is briefly unreadable, the one-time setup code the web + | installer asks for, and the lock that serialises installations. Outside + | public/, and readable only from the server. + */ + 'install' => [ + 'state_path' => env('DOCKET_INSTALL_STATE_PATH') ?: storage_path('app/install'), ], /* @@ -53,16 +63,24 @@ ], /* - | Inline images are downloaded to private storage so saved articles - | survive link rot. SVG is never stored locally: it can carry scripts and - | the copies are served from this origin. + | Inline and lead images are downloaded to private storage so saved + | articles survive link rot. SVG is never stored locally: it can carry + | scripts and the copies are served from this origin. Every limit below + | is per article and counts attempts, not just successes, so a page full + | of broken images can't keep a worker busy; images past a limit keep + | their remote src. */ 'images' => [ 'enabled' => true, // Per image, in kilobytes (5 MB). 'max_kb' => 5120, - // Images per article; the rest keep their remote src. + // Download attempts per article, lead image included. 'max_per_item' => 40, + // Total downloaded per article, in kilobytes (25 MB). + 'max_total_kb' => 25600, + // Seconds an article may spend downloading images, kept well inside + // the fetch job's 300-second timeout. + 'time_budget' => 120, ], /* diff --git a/config/queue.php b/config/queue.php index 79c2c0a..20d5345 100644 --- a/config/queue.php +++ b/config/queue.php @@ -40,7 +40,7 @@ 'connection' => env('DB_QUEUE_CONNECTION'), 'table' => env('DB_QUEUE_TABLE', 'jobs'), 'queue' => env('DB_QUEUE', 'default'), - 'retry_after' => (int) env('DB_QUEUE_RETRY_AFTER', 90), + 'retry_after' => (int) env('DB_QUEUE_RETRY_AFTER', 360), 'after_commit' => false, ], @@ -48,7 +48,7 @@ 'driver' => 'beanstalkd', 'host' => env('BEANSTALKD_QUEUE_HOST', 'localhost'), 'queue' => env('BEANSTALKD_QUEUE', 'default'), - 'retry_after' => (int) env('BEANSTALKD_QUEUE_RETRY_AFTER', 90), + 'retry_after' => (int) env('BEANSTALKD_QUEUE_RETRY_AFTER', 360), 'block_for' => 0, 'after_commit' => false, ], @@ -68,7 +68,7 @@ 'driver' => 'redis', 'connection' => env('REDIS_QUEUE_CONNECTION', 'default'), 'queue' => env('REDIS_QUEUE', 'default'), - 'retry_after' => (int) env('REDIS_QUEUE_RETRY_AFTER', 90), + 'retry_after' => (int) env('REDIS_QUEUE_RETRY_AFTER', 360), 'block_for' => null, 'after_commit' => false, ], diff --git a/database/seeders/DatabaseSeeder.php b/database/seeders/DatabaseSeeder.php index cb7fe79..a0087b3 100644 --- a/database/seeders/DatabaseSeeder.php +++ b/database/seeders/DatabaseSeeder.php @@ -5,19 +5,33 @@ use App\Models\User; use Illuminate\Database\Seeder; use Illuminate\Support\Facades\Hash; +use RuntimeException; class DatabaseSeeder extends Seeder { public function run(): void { - // Single-user app: one seeded account, no registration flow. - // Set DOCKET_USER_PASSWORD in .env before seeding in production. - User::firstOrCreate( - ['email' => config('docket.user.email')], - [ - 'name' => config('docket.user.name'), - 'password' => Hash::make(config('docket.user.password')), - ] - ); + // Single-user app. The installer creates the owner; this exists for + // rebuilding an empty database and never adds a second account, + // whatever email the existing one now has. + if (User::query()->exists()) { + $this->command->info('An owner account already exists, so no user was seeded.'); + + return; + } + + $password = config('docket.user.password'); + + // No published fallback: a seeded account with a known password + // would open the whole library to anyone who read the source. + if (! is_string($password) || $password === '') { + throw new RuntimeException('Set DOCKET_USER_PASSWORD in .env before seeding, or run php artisan docket:install.'); + } + + User::query()->create([ + 'name' => config('docket.user.name'), + 'email' => config('docket.user.email'), + 'password' => Hash::make($password), + ]); } } diff --git a/resources/views/import/index.blade.php b/resources/views/import/index.blade.php index c7fef07..5540994 100644 --- a/resources/views/import/index.blade.php +++ b/resources/views/import/index.blade.php @@ -37,6 +37,10 @@ class="mt-1 block w-full text-sm text-slate-700 file:mr-4 file:rounded-md file:b {{ __('Download CSV') }} — {{ __('the same columns as Pocket\'s export, so it imports back into Docket or anything that reads Pocket files.') }} +
  • + {{ __('Download CSV for spreadsheets') }} + — {{ __('the same columns, safe to open in Excel, Numbers or Sheets: any title or tag that starts like a formula is kept as plain text (with a leading apostrophe), so a hostile page title can\'t run as a formula. Use the plain CSV for importing.') }} +
  • {{ __('Download JSON') }} — {{ __('everything, including the saved article text and HTML.') }} diff --git a/resources/views/install/setup.blade.php b/resources/views/install/setup.blade.php index feb3d5c..7bb04d5 100644 --- a/resources/views/install/setup.blade.php +++ b/resources/views/install/setup.blade.php @@ -14,11 +14,24 @@ @endif {{-- No @csrf: these routes run before the session store exists and are - disabled entirely once the owner account has been created. + disabled entirely once the owner account has been created. The + setup code does the job CSRF would: a request that can't read the + server's files can't supply it. Relative action, not route(): before .env exists the app defaults to APP_ENV=production and forces https URLs, which breaks installs served over plain http. --}}
    + + + + Proves you control the server. Read it there: + cat {{ $codeLocation }} + + @if ($errors->has('setup_code')) +

    {{ $errors->first('setup_code') }}

    + @endif + diff --git a/resources/views/install/unavailable.blade.php b/resources/views/install/unavailable.blade.php new file mode 100644 index 0000000..5cace7b --- /dev/null +++ b/resources/views/install/unavailable.blade.php @@ -0,0 +1,11 @@ +@extends('install.layout') + +@section('content') +

    The installer is unavailable

    + +

    + Docket couldn't confirm whether it is already installed, so it won't + offer to install itself. Fix the database connection (or its file + permissions), then reload this page. +

    +@endsection diff --git a/resources/views/items/partials/row.blade.php b/resources/views/items/partials/row.blade.php index b7fc64e..cdde0c1 100644 --- a/resources/views/items/partials/row.blade.php +++ b/resources/views/items/partials/row.blade.php @@ -1,6 +1,9 @@ @php /** @var \App\Models\Item $item */ $failed = $item->fetch_status === \App\Enums\FetchStatus::Failed; + // fetched_at is only set by a successful fetch, so a failed item that + // has one still has its saved copy (lists don't load article bodies). + $refreshFailed = $failed && $item->fetched_at !== null; $pending = $item->fetch_status === \App\Enums\FetchStatus::Pending; @endphp
  • @@ -33,7 +36,7 @@ class="rounded border-slate-300 text-teal-700 focus:ring-teal-500"> {{ __('Fetching…') }} @elseif ($failed) - {{ __('Could not fetch') }} + {{ $refreshFailed ? __('Refresh failed') : __('Could not fetch') }} @endif @if ($item->isArchived()) @@ -45,7 +48,7 @@ class="rounded border-slate-300 text-teal-700 focus:ring-teal-500">

    {{ $item->excerpt }}

    @endif - @if ($failed && $item->fetch_error) + @if ($failed && ! $refreshFailed && $item->fetch_error)

    {{ $item->fetch_error }}

    @endif diff --git a/resources/views/items/show.blade.php b/resources/views/items/show.blade.php index 1aa1730..abaa7cb 100644 --- a/resources/views/items/show.blade.php +++ b/resources/views/items/show.blade.php @@ -149,6 +149,14 @@ class="reader-panel absolute right-0 mt-2 w-64 space-y-4 rounded-lg border p-4 s @if ($item->hasArticle()) + @if ($item->refreshFailed()) +

    + {{ __('Could not refresh this article: :error You are reading the copy saved on :date.', [ + 'error' => rtrim((string) $item->fetch_error, '.').'.', + 'date' => $item->fetched_at?->format('j F Y') ?? __('an earlier visit'), + ]) }} +

    + @endif
    {!! $item->content_html !!}
    diff --git a/routes/web.php b/routes/web.php index 20a7b5c..eafce07 100644 --- a/routes/web.php +++ b/routes/web.php @@ -23,7 +23,7 @@ function () { Route::get('archive', [ItemController::class, 'index'])->defaults('view', 'archive')->name('items.archive'); Route::get('favourites', [ItemController::class, 'index'])->defaults('view', 'favourites')->name('items.favourites'); Route::get('search', [ItemController::class, 'index'])->defaults('view', 'search')->name('items.search'); - Route::get('tags/{tag}', [ItemController::class, 'tagged'])->name('items.tagged'); + Route::get('tags/{tag}', [ItemController::class, 'tagged'])->whereNumber('tag')->name('items.tagged'); Route::post('items', [ItemController::class, 'store'])->name('items.store'); Route::post('items/bulk', BulkItemController::class)->name('items.bulk'); @@ -48,8 +48,8 @@ function () { Route::get('share', [SaveController::class, 'share'])->name('save.share'); Route::get('tags', [TagController::class, 'index'])->name('tags.index'); - Route::patch('tags/{tag}', [TagController::class, 'update'])->name('tags.update'); - Route::delete('tags/{tag}', [TagController::class, 'destroy'])->name('tags.destroy'); + Route::patch('tags/{tag}', [TagController::class, 'update'])->whereNumber('tag')->name('tags.update'); + Route::delete('tags/{tag}', [TagController::class, 'destroy'])->whereNumber('tag')->name('tags.destroy'); Route::get('import', [ImportController::class, 'index'])->name('import.index'); Route::post('import', [ImportController::class, 'store'])->name('import.store'); diff --git a/tests/Feature/ContentSecurityPolicyTest.php b/tests/Feature/ContentSecurityPolicyTest.php index faddbb6..d63d7cc 100644 --- a/tests/Feature/ContentSecurityPolicyTest.php +++ b/tests/Feature/ContentSecurityPolicyTest.php @@ -94,7 +94,7 @@ public function test_no_page_relies_on_inline_handlers_or_style_attributes(): vo $user->createToken('Shortcut'); $pages = [ - '/', '/archive', '/favourites', '/search?q=the', '/tags', '/tags/reading', + '/', '/archive', '/favourites', '/search?q=the', '/tags', "/tags/{$tag->id}", "/items/{$item->id}", '/import', '/settings', '/tools', '/profile', '/save?url='.urlencode('https://example.com/new'), ]; diff --git a/tests/Feature/DatabaseSeederTest.php b/tests/Feature/DatabaseSeederTest.php new file mode 100644 index 0000000..ba49f1d --- /dev/null +++ b/tests/Feature/DatabaseSeederTest.php @@ -0,0 +1,57 @@ + 'old-owner@example.com', 'docket.user.password' => 'some-password-1']); + User::factory()->create(['email' => 'new-owner@example.com']); + + Artisan::call('db:seed', ['--class' => DatabaseSeeder::class, '--force' => true]); + + $this->assertSame(['new-owner@example.com'], User::query()->pluck('email')->all()); + } + + public function test_seeding_an_empty_database_needs_an_explicit_password(): void + { + config(['docket.user.password' => null]); + + try { + Artisan::call('db:seed', ['--class' => DatabaseSeeder::class, '--force' => true]); + $this->fail('Seeding without DOCKET_USER_PASSWORD should refuse.'); + } catch (RuntimeException $e) { + $this->assertStringContainsString('DOCKET_USER_PASSWORD', $e->getMessage()); + } + + $this->assertSame(0, User::query()->count()); + } + + public function test_seeding_an_empty_database_with_a_password(): void + { + config([ + 'docket.user.name' => 'Owner', + 'docket.user.email' => 'owner@example.com', + 'docket.user.password' => 'chosen-password-1', + ]); + + Artisan::call('db:seed', ['--class' => DatabaseSeeder::class, '--force' => true]); + + $user = User::query()->sole(); + $this->assertSame('owner@example.com', $user->email); + $this->assertTrue(Hash::check('chosen-password-1', $user->password)); + } +} diff --git a/tests/Feature/HardeningTest.php b/tests/Feature/HardeningTest.php new file mode 100644 index 0000000..61b1129 --- /dev/null +++ b/tests/Feature/HardeningTest.php @@ -0,0 +1,274 @@ +create($attributes + [ + 'url' => $url, + 'normalized_url' => app(UrlNormaliser::class)->normalise($url), + 'title' => null, + ]); + } + + private function articleHtml(string $body = 'A readable paragraph about reading and libraries.'): string + { + return 'Article' + .'
    ' + .str_repeat('

    '.$body.' '.str_repeat('More words to make a proper paragraph. ', 8).'

    ', 6) + .'
    '; + } + + // Queue timing -------------------------------------------------------- + + public function test_queue_reservations_outlast_the_fetch_job(): void + { + $timeout = (new FetchItemContent($this->item()))->timeout; + + foreach (['database', 'redis', 'beanstalkd'] as $connection) { + $this->assertGreaterThan($timeout, config("queue.connections.{$connection}.retry_after"), $connection); + } + } + + public function test_the_same_item_is_never_fetched_twice_at_once(): void + { + Http::fake(); + $item = $this->item(); + $job = new FetchItemContent($item); + + // Another worker is mid-fetch on this item. + $lock = Cache::lock($job->middleware()[0]->getLockKey($job), 600); + $this->assertTrue($lock->get()); + + FetchItemContent::dispatchSync($item); + + Http::assertNothingSent(); + $this->assertSame(0, $item->refresh()->fetch_attempts); + + $lock->release(); + } + + // Memory --------------------------------------------------------------- + + public function test_lists_bulk_actions_and_csv_export_never_select_article_bodies(): void + { + Queue::fake(); + $user = User::factory()->create(); + $items = Item::factory()->count(3)->fetched()->create(); + + DB::enableQueryLog(); + + $this->actingAs($user)->get('/')->assertOk(); + $this->actingAs($user)->get('/search?q=the')->assertOk(); + $this->actingAs($user)->post('/items/bulk', ['action' => 'archive', 'ids' => $items->pluck('id')->all()]); + $this->actingAs($user)->get('/export/csv')->streamedContent(); + + $bodies = collect(DB::getQueryLog()) + ->pluck('query') + ->filter(fn (string $sql): bool => str_starts_with(strtolower($sql), 'select') + && str_contains($sql, '"items"') + && (str_contains($sql, '"items".*') || str_contains($sql, 'select *') || str_contains($sql, 'content_html'))); + + $this->assertSame([], $bodies->values()->all()); + $this->assertSame(3, Item::query()->archived()->count()); + } + + // Images --------------------------------------------------------------- + + public function test_failed_image_downloads_use_up_the_per_article_limit(): void + { + config(['docket.images.max_per_item' => 2]); + Http::fake(['*' => Http::response('', 404)]); + + $html = ''; + for ($i = 0; $i < 7; $i++) { + $html .= ''; + } + + app(ImageLocaliser::class)->localise($this->item(), $html); + + Http::assertSentCount(2); + } + + public function test_image_downloads_stop_at_the_per_article_byte_budget(): void + { + Storage::fake(Item::DISK); + config(['docket.images.max_total_kb' => 1]); + Http::fake(['*' => Http::response(str_repeat('x', 900))]); + + app(ImageLocaliser::class)->localise($this->item(), ''); + + // 900 bytes, then 124 left (capped), then nothing left to spend. + Http::assertSentCount(2); + } + + public function test_disabled_downloads_skip_the_lead_image_too(): void + { + Storage::fake(Item::DISK); + config(['docket.images.enabled' => false]); + Http::fake([ + 'https://example.com/article' => Http::response($this->articleHtml(), 200, ['Content-Type' => 'text/html']), + '*' => Http::response((string) base64_decode(self::PNG)), + ]); + $item = $this->item(); + + FetchItemContent::dispatchSync($item); + + $item->refresh(); + $this->assertSame(FetchStatus::Fetched, $item->fetch_status); + $this->assertNull($item->lead_image_path); + $this->assertSame('https://cdn.example.com/lead.png', $item->lead_image_url); + Http::assertSentCount(1); + } + + public function test_a_lead_image_shown_inline_is_downloaded_once(): void + { + Storage::fake(Item::DISK); + $html = str_replace('
    ', '

    ', $this->articleHtml()); + Http::fake([ + 'https://example.com/article' => Http::response($html, 200, ['Content-Type' => 'text/html']), + 'https://cdn.example.com/lead.png' => Http::response((string) base64_decode(self::PNG)), + ]); + $item = $this->item(); + + FetchItemContent::dispatchSync($item); + + $this->assertSame(sha1('https://cdn.example.com/lead.png').'.png', $item->refresh()->lead_image_path); + Http::assertSentCount(2); + } + + public function test_a_failed_image_write_keeps_the_remote_address(): void + { + Http::fake(['*' => Http::response((string) base64_decode(self::PNG))]); + $disk = Mockery::mock(Filesystem::class); + $disk->shouldReceive('put')->once()->andReturn(false); + Storage::shouldReceive('disk')->with(Item::DISK)->andReturn($disk); + + $item = $this->item(); + $html = app(ImageLocaliser::class)->localise($item, ''); + + $this->assertStringContainsString('src="https://example.com/image.png"', $html); + $this->assertStringNotContainsString('/items/'.$item->id.'/images/', $html); + } + + // Refresh failures ----------------------------------------------------- + + public function test_a_failed_refresh_keeps_the_saved_article_readable(): void + { + Http::fake(['*' => Http::response('', 404)]); + $item = Item::factory()->fetched('The preserved article body.')->create([ + 'author' => 'Original Author', + 'site_name' => 'Original Site', + ]); + + FetchItemContent::dispatchSync($item); + $item->refresh(); + + $this->assertSame(FetchStatus::Failed, $item->fetch_status); + $this->assertTrue($item->hasArticle()); + $this->assertTrue($item->refreshFailed()); + $this->assertSame('Original Author', $item->author); + + $this->actingAs(User::factory()->create()) + ->get("/items/{$item->id}") + ->assertOk() + ->assertSee('The preserved article body.') + ->assertSee('Could not refresh this article: The site responded with HTTP 404.'); + + $this->actingAs(User::factory()->create())->get('/')->assertSee('Refresh failed'); + } + + public function test_a_refresh_that_finds_no_article_keeps_the_saved_details(): void + { + Http::fake(['*' => Http::response('Gone', 200, ['Content-Type' => 'text/html'])]); + $item = Item::factory()->fetched('Still here.')->create([ + 'title' => 'Original title', + 'excerpt' => 'Original excerpt', + 'site_name' => 'Original Site', + ]); + + FetchItemContent::dispatchSync($item); + $item->refresh(); + + $this->assertSame('Original title', $item->title); + $this->assertSame('Original excerpt', $item->excerpt); + $this->assertSame('Original Site', $item->site_name); + $this->assertStringContainsString('Still here.', (string) $item->content_html); + } + + // Refetch paging ------------------------------------------------------- + + public function test_refetch_reaches_every_item_as_the_selection_shrinks(): void + { + // On a sync queue each item is fetched (and leaves the failed set) + // during the loop; offset paging would skip the rows behind it. + config(['queue.default' => 'sync', 'docket.import.spacing_seconds' => 0, 'docket.images.enabled' => false]); + Http::fake(fn () => Http::response($this->articleHtml(), 200, ['Content-Type' => 'text/html'])); + + Item::factory()->count(450)->failed()->create(['fetch_attempts' => 0, 'title' => 'Kept']); + + $this->artisan('docket:refetch')->assertSuccessful(); + + $this->assertSame(0, Item::query()->where('fetch_attempts', 0)->count()); + $this->assertSame(450, Item::query()->where('fetch_status', FetchStatus::Fetched->value)->count()); + } + + // Export --------------------------------------------------------------- + + public function test_the_spreadsheet_csv_neutralises_formulas_and_the_plain_one_is_lossless(): void + { + Item::factory()->create(['title' => '=HYPERLINK("https://evil.example","x")', 'url' => 'https://example.com/a']); + Item::factory()->create(['title' => " \t@SUM(A1)", 'url' => 'https://example.com/b']); + Item::factory()->create(['title' => 'An ordinary title', 'url' => 'https://example.com/c']); + + $user = User::factory()->create(); + $parse = fn (string $csv): array => array_map( + fn (string $line): array => str_getcsv($line, ',', '"', ''), + array_values(array_filter(explode("\n", $csv))) + ); + + $plain = $parse($this->actingAs($user)->get('/export/csv')->streamedContent()); + $safe = $this->actingAs($user)->get('/export/csv?for=spreadsheet'); + $this->assertStringContainsString('-spreadsheet.csv', (string) $safe->headers->get('Content-Disposition')); + $safe = $parse($safe->streamedContent()); + + $this->assertSame('=HYPERLINK("https://evil.example","x")', $plain[1][0]); + $this->assertSame('\'=HYPERLINK("https://evil.example","x")', $safe[1][0]); + $this->assertSame("' \t@SUM(A1)", $safe[2][0]); + $this->assertSame('An ordinary title', $safe[3][0]); + $this->assertSame('https://example.com/a', $safe[1][1]); + + foreach (['-1', '+1', '@x', "\r=1", "\x01=1"] as $dangerous) { + $this->assertStringStartsWith("'", Exporter::spreadsheetSafe($dangerous), json_encode($dangerous) ?: ''); + } + } +} diff --git a/tests/Feature/InstallCommandTest.php b/tests/Feature/InstallCommandTest.php index c8c4ce6..c1fff5a 100644 --- a/tests/Feature/InstallCommandTest.php +++ b/tests/Feature/InstallCommandTest.php @@ -142,4 +142,32 @@ public function test_rejects_short_password(): void $this->assertSame(0, User::query()->count()); } + + public function test_force_never_adds_a_second_account_under_another_email(): void + { + User::factory()->create(['email' => 'owner@example.com']); + + $this->artisan('docket:install', [ + '--url' => 'http://localhost', + '--name' => 'Someone Else', + '--email' => 'someone@example.com', + '--password' => 'another-password-1', + '--force' => true, + ])->assertFailed(); + + $this->assertSame(['owner@example.com'], User::query()->pluck('email')->all()); + $this->assertStringNotContainsString('someone@example.com', $this->envContents()); + } + + public function test_installing_leaves_the_installed_marker(): void + { + $this->artisan('docket:install', [ + '--url' => 'http://localhost', + '--email' => 'jane@example.com', + '--password' => 'super-secret-99', + '--no-interaction' => true, + ])->assertSuccessful(); + + $this->assertFileExists($this->installState.'/installed'); + } } diff --git a/tests/Feature/Items/OrganisingTest.php b/tests/Feature/Items/OrganisingTest.php index 009a551..89447a5 100644 --- a/tests/Feature/Items/OrganisingTest.php +++ b/tests/Feature/Items/OrganisingTest.php @@ -109,7 +109,8 @@ public function test_tags_are_added_in_the_reader_and_listed(): void $this->assertSame('Renamed', $item->title); $this->assertSame(['Laravel', 'php'], $item->tags->pluck('name')->all()); - $this->actingAs($this->user)->get('/tags/laravel')->assertOk()->assertSee('Renamed'); + $laravel = Tag::query()->where('slug', 'laravel')->sole(); + $this->actingAs($this->user)->get("/tags/{$laravel->id}")->assertOk()->assertSee('Renamed'); $this->actingAs($this->user)->get('/tags')->assertOk()->assertSee('Laravel')->assertSee('1 item'); // Clearing the field removes every tag (the tags themselves stay). @@ -126,17 +127,17 @@ public function test_tags_can_be_renamed_merged_and_deleted(): void $b->attachTagNames(['JavaScript']); $js = Tag::query()->where('slug', 'js')->sole(); - $this->actingAs($this->user)->patch("/tags/{$js->slug}", ['name' => 'javascript'])->assertRedirect('/tags'); + $this->actingAs($this->user)->patch("/tags/{$js->id}", ['name' => 'javascript'])->assertRedirect('/tags'); $this->assertModelMissing($js); $merged = Tag::query()->where('slug', 'javascript')->sole(); $this->assertSame(2, $merged->items()->count()); - $this->actingAs($this->user)->patch("/tags/{$merged->slug}", ['name' => 'Web dev']); + $this->actingAs($this->user)->patch("/tags/{$merged->id}", ['name' => 'Web dev']); $this->assertSame('Web dev', $merged->refresh()->name); $this->assertSame('web-dev', $merged->slug); - $this->actingAs($this->user)->delete('/tags/web-dev')->assertRedirect('/tags'); + $this->actingAs($this->user)->delete("/tags/{$merged->id}")->assertRedirect('/tags'); $this->assertSame(0, Tag::query()->count()); $this->assertSame(2, Item::query()->count()); } @@ -213,4 +214,17 @@ public function test_deleting_a_single_item(): void $other = Item::factory()->create(); $this->actingAs($this->user)->from("/items/{$other->id}")->delete("/items/{$other->id}")->assertRedirect('/'); } + + public function test_tags_with_url_characters_in_their_names_still_open(): void + { + $item = Item::factory()->create(['title' => 'Slashed']); + + foreach (['work/personal', 'C# & .NET', '50% off?', 'naïve café'] as $name) { + $item->syncTagNames([$name]); + $tag = Tag::query()->where('slug', Tag::slugFor($name))->sole(); + + $this->actingAs($this->user)->get(route('items.tagged', $tag))->assertOk()->assertSee('Slashed'); + $this->actingAs($this->user)->patch(route('tags.update', $tag), ['name' => $name.' 2'])->assertRedirect('/tags'); + } + } } diff --git a/tests/Feature/Items/RealTransportTest.php b/tests/Feature/Items/RealTransportTest.php new file mode 100644 index 0000000..a981c6a --- /dev/null +++ b/tests/Feature/Items/RealTransportTest.php @@ -0,0 +1,139 @@ +markTestSkipped('The curl extension is required.'); + } + + $this->port = $this->freePort(); + $this->server = new Process([PHP_BINARY, '-S', "127.0.0.1:{$this->port}", base_path('tests/Fixtures/server/router.php')]); + $this->server->start(); + $this->waitForServer(); + + Http::allowStrayRequests(); + $this->dns->set(self::HOST, '127.0.0.1'); + $this->app->instance(IpGuard::class, new class() extends IpGuard + { + public function allows(string $ip): bool + { + return $ip === '127.0.0.1'; + } + }); + } + + protected function tearDown(): void + { + $this->server->stop(0); + + parent::tearDown(); + } + + private function url(string $path): string + { + return 'http://'.self::HOST.':'.$this->port.$path; + } + + public function test_a_hostname_is_fetched_through_the_pinned_address(): void + { + $response = app(SafeHttpClient::class)->get($this->url('/article'), 10_000); + + $this->assertSame(200, $response->status); + $this->assertSame('text/html', $response->contentType); + $this->assertSame('utf-8', $response->charset); + // The Host header is still the hostname: only the connection is pinned. + $this->assertStringContainsString('Served to '.self::HOST.':'.$this->port, $response->body); + } + + public function test_redirects_are_followed_over_the_real_transport(): void + { + $response = app(SafeHttpClient::class)->get($this->url('/redirect'), 10_000); + + $this->assertSame($this->url('/article'), $response->url); + } + + public function test_the_size_cap_aborts_a_declared_oversized_body(): void + { + $this->expectException(FetchFailedException::class); + $this->expectExceptionMessage('larger than'); + + app(SafeHttpClient::class)->get($this->url('/declared-large'), 10_000); + } + + public function test_the_size_cap_aborts_while_an_undeclared_body_downloads(): void + { + $this->expectException(FetchFailedException::class); + $this->expectExceptionMessage('larger than'); + + app(SafeHttpClient::class)->get($this->url('/streamed-large'), 10_000); + } + + public function test_a_hostname_on_a_blocked_address_never_connects(): void + { + $this->app->instance(IpGuard::class, new IpGuard()); + + $this->expectException(FetchFailedException::class); + $this->expectExceptionMessage('private or local network address'); + + app(SafeHttpClient::class)->get($this->url('/article'), 10_000); + } + + private function freePort(): int + { + $socket = stream_socket_server('tcp://127.0.0.1:0', $errno, $error); + $this->assertNotFalse($socket, $error); + $name = (string) stream_socket_get_name($socket, false); + fclose($socket); + + return (int) substr($name, strrpos($name, ':') + 1); + } + + private function waitForServer(): void + { + for ($i = 0; $i < 50; $i++) { + $connection = @fsockopen('127.0.0.1', $this->port, $errno, $error, 0.1); + + if ($connection !== false) { + fclose($connection); + + return; + } + + usleep(100_000); + } + + $this->fail('The fixture server did not start: '.$this->server->getErrorOutput()); + } +} diff --git a/tests/Feature/PageSmokeTest.php b/tests/Feature/PageSmokeTest.php index 745a767..da56482 100644 --- a/tests/Feature/PageSmokeTest.php +++ b/tests/Feature/PageSmokeTest.php @@ -35,7 +35,7 @@ public function test_all_pages_render(): void '/search?q=the&sort=newest', '/search', '/tags', - '/tags/reading', + "/tags/{$tag->id}", "/items/{$fetched->id}", "/items/{$pending->id}", "/items/{$failed->id}", diff --git a/tests/Feature/WebInstallerTest.php b/tests/Feature/WebInstallerTest.php index 374594f..5a9d89d 100644 --- a/tests/Feature/WebInstallerTest.php +++ b/tests/Feature/WebInstallerTest.php @@ -3,10 +3,14 @@ namespace Tests\Feature; use App\Models\User; +use App\Services\Installer; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Hash; +use Illuminate\Support\Facades\Schema; use Illuminate\Support\Facades\URL; use Illuminate\Support\Str; +use Mockery; +use RuntimeException; use Tests\TestCase; class WebInstallerTest extends TestCase @@ -43,6 +47,7 @@ protected function tearDown(): void private function validPayload(): array { return [ + 'setup_code' => app(Installer::class)->setupCode(), 'url' => 'https://docket.example.com', 'name' => 'Jane Doe', 'email' => 'jane@example.com', @@ -145,4 +150,87 @@ public function test_install_routes_do_not_require_csrf_or_session(): void // outside the 'web' group because no session store exists yet. $this->post('/install/setup', [])->assertStatus(422); } + + public function test_the_setup_form_says_where_to_find_the_setup_code(): void + { + $this->get('/install/setup') + ->assertOk() + ->assertSee('Setup code') + ->assertSee('cat '.app(Installer::class)->setupCodeLocation()); + + $this->assertFileExists($this->installState.'/setup-code'); + } + + public function test_a_wrong_or_missing_setup_code_is_refused(): void + { + foreach (['', 'aaaa-bbbb-cccc-dddd'] as $code) { + $this->post('/install/setup', ['setup_code' => $code] + $this->validPayload()) + ->assertStatus(422) + ->assertSee('Setup code'); + } + + $this->assertSame(0, User::query()->count()); + $this->assertFileDoesNotExist($this->envDir.'/.env'); + } + + public function test_installing_retires_the_setup_code_and_leaves_a_marker(): void + { + $this->post('/install/setup', $this->validPayload())->assertOk(); + + $this->assertFileDoesNotExist($this->installState.'/setup-code'); + $this->assertFileExists($this->installState.'/installed'); + } + + public function test_the_marker_keeps_the_installer_closed_even_with_no_accounts(): void + { + app(Installer::class)->markInstalled(); + + $this->get('/install')->assertRedirect(route('login')); + $this->post('/install/setup', $this->validPayload())->assertRedirect(route('login')); + $this->assertSame(0, User::query()->count()); + } + + public function test_concurrent_installs_cannot_create_two_owners(): void + { + // Pause the first installation just before it creates its account + // and run a complete second one in that window, as two overlapping + // requests would. The second is refused by the lock. + $real = new Installer($this->app); + $installer = Mockery::mock(Installer::class, [$this->app])->makePartial(); + $second = null; + + $installer->shouldReceive('createOwner')->andReturnUsing(function (string $name, string $email, string $password) use (&$second, $real) { + $second ??= $this->post('/install/setup', array_replace($this->validPayload(), ['email' => 'intruder@example.com'])); + + return $real->createOwner($name, $email, $password); + }); + $this->app->instance(Installer::class, $installer); + + $this->post('/install/setup', $this->validPayload())->assertOk()->assertSee('Docket is installed'); + + $this->assertNotNull($second); + $second->assertStatus(409)->assertSee('Another installation is running'); + $this->assertSame(['jane@example.com'], User::query()->pluck('email')->all()); + } + + public function test_a_second_owner_is_never_created_even_past_the_checks(): void + { + User::factory()->create(); + + $this->expectException(RuntimeException::class); + + app(Installer::class)->createOwner('Someone', 'someone@example.com', 'password-123'); + } + + public function test_an_unreadable_database_keeps_the_installer_closed(): void + { + // A configured database that errors is not a fresh install: fail + // closed, and touch nothing. + Schema::shouldReceive('hasTable')->andThrow(new RuntimeException('database is locked')); + + $this->get('/install')->assertStatus(503)->assertSee('installer stays closed'); + $this->post('/install/setup', $this->validPayload())->assertStatus(503); + + $this->assertFileDoesNotExist($this->envDir.'/.env'); + } } diff --git a/tests/Fixtures/server/router.php b/tests/Fixtures/server/router.php new file mode 100644 index 0000000..df496c6 --- /dev/null +++ b/tests/Fixtures/server/router.php @@ -0,0 +1,36 @@ +Fixture

    Served to ' + .htmlspecialchars($_SERVER['HTTP_HOST'] ?? '').'

    '; + break; + + case '/redirect': + header('Location: /article', true, 302); + break; + + case '/declared-large': + header('Content-Type: text/html'); + header('Content-Length: 50000'); + echo str_repeat('a', 50000); + break; + + case '/streamed-large': + // No Content-Length: the cap has to trip while the body arrives. + header('Content-Type: text/html'); + for ($i = 0; $i < 50; $i++) { + echo str_repeat('b', 1000); + flush(); + } + break; + + default: + http_response_code(404); +} diff --git a/tests/TestCase.php b/tests/TestCase.php index be42ace..4ae4145 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -4,13 +4,21 @@ use App\Services\Fetching\HostResolver; use Illuminate\Foundation\Testing\TestCase as BaseTestCase; +use Illuminate\Support\Facades\File; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Str; use Tests\Support\FakeHostResolver; abstract class TestCase extends BaseTestCase { protected FakeHostResolver $dns; + /** + * Per-test installer state (marker, setup code, lock), so no test sees + * another's installation and none touches storage/app/install. + */ + protected string $installState; + protected function setUp(): void { parent::setUp(); @@ -22,5 +30,15 @@ protected function setUp(): void $this->app->instance(HostResolver::class, $this->dns); Http::preventStrayRequests(); + + $this->installState = sys_get_temp_dir().'/docket-install-state-'.Str::random(10); + config(['docket.install.state_path' => $this->installState]); + } + + protected function tearDown(): void + { + File::deleteDirectory($this->installState); + + parent::tearDown(); } }