From 8f12c83d21bdd540670b33b5c108a2d27c905271 Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Fri, 21 Aug 2026 14:35:49 -0300 Subject: [PATCH] Tell a clone-and-build install to rebuild, not to pull MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ProjectSend prints the update instructions for the way this server was installed, and it knew two answers where it needed three: anything inside a container was handed `docker compose pull && docker compose up -d`. On the Compose stack that builds from a checkout there is no image behind those containers, so `pull` skips every ProjectSend service and `up -d` then finds them all current — the update reports success, changes nothing, and the dashboard goes on offering the same release. Reported by @mueller7382, who stayed on 2.0.0 that way while 2.1.0 was out (#1661). Those installations are now their own kind, told to `git pull` and rebuild, with the two steps a checkout needs that an image does not: its dependencies and its compiled frontend live outside git, so a release that moved either leaves them stale. Two signals decide it, in that order. The published image now declares itself with PROJECTSEND_IMAGE, which is the only evidence an operator bind-mounting over /var/www/html can neither hide nor forge; failing that — images published before this — a working tree in the install directory, which the image never has and the repository's own stack always does. getenv() rather than env(), because a cached configuration makes env() outside a config file return null, and the answer would flip silently on exactly the installs most likely to have cached it. The stale-code banner keeps treating both container kinds alike: what clears it is recreating the container, whichever way its image was built. The changelog also credits the reporter of #1663, which was missed when that entry was written. --- CHANGELOG.md | 14 +++- .../Platform/Installation/Installation.php | 79 ++++++++++++++----- .../Installation/InstallationKind.php | 10 ++- docker/production/Dockerfile | 8 ++ .../js/components/code-notice-banner.tsx | 6 +- .../js/components/update-instructions.tsx | 37 ++++++++- .../Feature/Platform/InstallationKindTest.php | 76 ++++++++++++++---- .../Feature/Platform/StaleCodeNoticeTest.php | 19 +++-- 8 files changed, 206 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9061bd2b..34cff02e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,17 @@ a version is cut. ### Fixed +- **An installation that builds its own containers is no longer told to pull.** ProjectSend prints + the update instructions for the way you installed it, and it had two answers where it needed + three: anything running in a container was handed `docker compose pull && docker compose up -d`, + including the Compose stack that builds from a checkout of the repository. There is no image + behind those containers to pull, so both commands ran, reported success and changed nothing — and + the dashboard went on offering the same release. Those installations are now recognised and given + `git pull && docker compose up -d --build` instead, with the two extra steps a checkout needs when + a release moves its dependencies or its frontend. + ([#1661](https://github.com/projectsend/projectsend/issues/1661), reported by + [@mueller7382](https://github.com/mueller7382)) + - **The dashboard no longer fails on shared hosting.** To decide which update instructions to print, ProjectSend asks whether it is running inside a container by looking for a file in the root of the filesystem. On shared hosting PHP is usually confined to your own directory, and looking outside it @@ -23,7 +34,8 @@ a version is cut. it always was: a server that keeps PHP inside a single directory is not our container image, and gets the manual update instructions, which is correct for shared hosting anyway. Nothing to change on your side, and no setting you would have been able to change if there were. - ([#1663](https://github.com/projectsend/projectsend/issues/1663)) + ([#1663](https://github.com/projectsend/projectsend/issues/1663), reported by + [@denkfabrik-li](https://github.com/denkfabrik-li)) ## 2.1.0 — 18 August 2026 diff --git a/app/Modules/Platform/Installation/Installation.php b/app/Modules/Platform/Installation/Installation.php index d3a5b9ef..49c1dcb7 100644 --- a/app/Modules/Platform/Installation/Installation.php +++ b/app/Modules/Platform/Installation/Installation.php @@ -5,35 +5,56 @@ declare(strict_types=1); namespace App\Modules\Platform\Installation; /** - * Whether this installation runs from a container image or from files on a - * server somebody administers directly. + * Whether this installation runs from a container image, from a container + * the operator builds themselves, or from files on a server somebody + * administers directly. * * It exists because the application tells administrators how to upgrade, and - * the two answers have nothing in common. A container is replaced — - * `docker compose pull && docker compose up -d`, with the entrypoint running - * the migrations on the way up. A manual install is a sequence somebody - * performs by hand: back up, take the site down, unpack the release over the - * directory, migrate, refresh the caches, bring it back (INSTALL.md). + * the three answers have nothing in common. A published container is + * replaced — `docker compose pull && docker compose up -d`, with the + * entrypoint running the migrations on the way up. A container built from a + * checkout has to be given new code and rebuilt. A manual install is a + * sequence somebody performs by hand: back up, take the site down, unpack + * the release over the directory, migrate, refresh the caches, bring it back + * (INSTALL.md). * - * Printing the container command to someone who installed from a zip is - * worse than printing nothing: it names a tool they do not have, for a stack - * they are not running, at the exact moment they are trying to do the right - * thing. That was the behaviour before this class existed — the command was - * a hardcoded string in two React components, written when Docker was the - * only supported path. + * Printing the wrong one of those is worse than printing nothing. For a + * manual install the container command names a tool they do not have, for a + * stack they are not running, at the exact moment they are trying to do the + * right thing — that was the behaviour before this class existed, when the + * command was a hardcoded string in two React components. For a stack built + * from a checkout it is worse still, because the command runs: `pull` skips + * services that have no image to pull and `up -d` then finds every container + * already current, so the update reports success and changes nothing, and + * the dashboard goes on offering the same release forever (#1661). * - * The detection is the presence of the file a container runtime leaves in - * the root filesystem. It is a deliberately conservative signal: something - * exotic enough to run neither Docker nor Podman is reported as a manual - * install, which is the safer wrong answer of the two — the manual - * instructions are steps a person follows and check for themselves, while - * the container command is one they would paste. + * Two signals, in order: + * + * 1. The published image sets PROJECTSEND_IMAGE. A positive marker set at + * build time is the only one a bind mount can neither forge nor hide. + * 2. Failing that — images published before that variable existed — a + * working tree in the install directory. The image is built from an + * unpacked release artifact and has none; the Compose stack in the + * repository bind-mounts the repository itself. + * + * Being in a container at all is the presence of the file a container + * runtime leaves in the root filesystem. It is a deliberately conservative + * signal: something exotic enough to run neither Docker nor Podman is + * reported as a manual install, which is the safer wrong answer of the + * three — the manual instructions are steps a person follows and checks for + * themselves, while the container commands are ones they would paste. */ class Installation { public function kind(): InstallationKind { - return $this->inContainer() ? InstallationKind::Container : InstallationKind::Manual; + if (! $this->inContainer()) { + return InstallationKind::Manual; + } + + return $this->builtFromSource() + ? InstallationKind::ContainerSource + : InstallationKind::Container; } /** @@ -54,4 +75,22 @@ class Installation // directory is not the container image. return @file_exists('/.dockerenv') || @file_exists('/run/.containerenv'); } + + /** + * Protected for the same reason as inContainer(), and answered the same + * way in tests. + */ + protected function builtFromSource(): bool + { + // getenv() rather than env(): once the configuration is cached, + // env() outside a config file returns null, and the answer would + // silently flip on the installs most likely to have cached it. + if (getenv('PROJECTSEND_IMAGE') === '1') { + return false; + } + + // A worktree checkout writes .git as a file rather than a + // directory, so ask whether it exists, not what it is. + return file_exists(base_path('.git')); + } } diff --git a/app/Modules/Platform/Installation/InstallationKind.php b/app/Modules/Platform/Installation/InstallationKind.php index 34bfed65..17dd0051 100644 --- a/app/Modules/Platform/Installation/InstallationKind.php +++ b/app/Modules/Platform/Installation/InstallationKind.php @@ -11,9 +11,17 @@ namespace App\Modules\Platform\Installation; */ enum InstallationKind: string { - /** Runs from an image: upgrading is pulling a new one. */ + /** Runs from the published image: upgrading is pulling a new one. */ case Container = 'container'; + /** + * Runs from a container the operator builds themselves, out of a + * checkout of the repository: upgrading is new code first, then a + * rebuild. Pulling does nothing here — there is no published image + * behind these containers to pull. + */ + case ContainerSource = 'container-source'; + /** Runs from files on a server someone administers: upgrading is INSTALL.md's sequence. */ case Manual = 'manual'; } diff --git a/docker/production/Dockerfile b/docker/production/Dockerfile index 6eb2b2d9..2a4fc048 100644 --- a/docker/production/Dockerfile +++ b/docker/production/Dockerfile @@ -126,6 +126,14 @@ RUN mkdir -p storage/app/files storage/framework/cache storage/framework/session # session and makes every encrypted column unreadable. VOLUME ["/var/www/html/storage"] +# How the application knows it is this image and not a container somebody +# built from a checkout — the two upgrade completely differently, and it is +# the application that prints the instructions. See Installation. It has to +# be set here rather than inferred at runtime: an operator who bind-mounts +# over /var/www/html can hide any file the image ships as its evidence, and +# an environment variable survives that. +ENV PROJECTSEND_IMAGE=1 + EXPOSE 80 # Laravel's health route (bootstrap/app.php: health: '/up'). Hitting it diff --git a/resources/js/components/code-notice-banner.tsx b/resources/js/components/code-notice-banner.tsx index ac6b506a..11b39991 100644 --- a/resources/js/components/code-notice-banner.tsx +++ b/resources/js/components/code-notice-banner.tsx @@ -2,6 +2,7 @@ import { usePage } from '@inertiajs/react'; import { AlertTriangle } from 'lucide-react'; import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert'; +import { isContainerInstall } from '@/components/update-instructions'; import { useTranslation } from '@/hooks/use-translation'; import { type SharedData } from '@/types'; @@ -27,7 +28,10 @@ export function CodeNoticeBanner() { return null; } - const container = notice.install_kind === 'container'; + // Both container kinds answer this the same way: what fixes it is + // recreating the container, whether its image came from a registry or + // from a build on this machine. + const container = isContainerInstall(notice.install_kind); // Stated as a fact first, cause second. A deliberate rollback lands // here too, and telling somebody to reload PHP-FPM when they meant to diff --git a/resources/js/components/update-instructions.tsx b/resources/js/components/update-instructions.tsx index cdf52312..016d7b20 100644 --- a/resources/js/components/update-instructions.tsx +++ b/resources/js/components/update-instructions.tsx @@ -2,7 +2,17 @@ import { UpdateOptionsDialog } from '@/components/update-options-dialog'; import { useTranslation } from '@/hooks/use-translation'; import { cn } from '@/lib/utils'; -export type InstallKind = 'container' | 'manual'; +export type InstallKind = 'container' | 'container-source' | 'manual'; + +/** + * Whether this installation runs in a container, whichever way it got + * there. Everything that is about the runtime — restarting it, recreating + * it — is the same answer for both container kinds; only the upgrade + * itself differs. + */ +export function isContainerInstall(kind: InstallKind): boolean { + return kind === 'container' || kind === 'container-source'; +} /** * How to actually apply an available update, for this server. @@ -15,6 +25,13 @@ export type InstallKind = 'container' | 'manual'; * have, for a stack they are not using, at the exact moment they were trying * to do the right thing. * + * The same string was wrong a second way, for containers this time: a stack + * built from a checkout has no image to pull, so both commands succeed, + * report success, and change nothing at all. That install then keeps being + * offered the same release for as long as its operator keeps following the + * instructions on this screen (#1661) — which is why 'container' and + * 'container-source' are told different things. + * * `compact` is for the dashboard card, a narrow column beside other widgets. * `codeClassName` exists for the same caller — its code sits inside a warning * alert and has to match it. @@ -44,6 +61,24 @@ export function UpdateInstructions({ ); } + if (kind === 'container-source') { + return ( + <> + {/* On its own line rather than inline like the image's one-liner: + this command is long enough to wrap inside the dashboard's + narrow column, and a command split across two lines mid-word + is one somebody retypes wrongly. */} +

{t('To update, run this in the directory you cloned:')}

+ git pull && docker compose up -d --build +

+ {t( + 'This installation builds its own images, so pulling one changes nothing. If the release moved composer.lock or the frontend, run composer install and npm run build as well — UPDATE.md has the full procedure.', + )} +

+ + ); + } + if (compact) { return (
diff --git a/tests/Feature/Platform/InstallationKindTest.php b/tests/Feature/Platform/InstallationKindTest.php index f85f0084..e2154380 100644 --- a/tests/Feature/Platform/InstallationKindTest.php +++ b/tests/Feature/Platform/InstallationKindTest.php @@ -11,19 +11,30 @@ use Inertia\Testing\AssertableInertia; /** * The application tells administrators how to upgrade, and the answer is - * different for a container and for files unpacked onto a server. Getting it - * wrong is worse than saying nothing: before this existed, both surfaces - * printed `docker compose pull` to everybody, including the people INSTALL.md - * was written for, who have no docker to run it with. + * different for the published image, for a container somebody builds from a + * checkout, and for files unpacked onto a server. Getting it wrong is worse + * than saying nothing: before this existed, every surface printed + * `docker compose pull` to everybody, including the people INSTALL.md was + * written for, who have no docker to run it with — and including the + * clone-and-build stacks where those commands succeed without updating + * anything (#1661). */ class FakeInstallation extends Installation { - public function __construct(private readonly bool $container) {} + public function __construct( + private readonly bool $container, + private readonly bool $source = false, + ) {} protected function inContainer(): bool { return $this->container; } + + protected function builtFromSource(): bool + { + return $this->source; + } } beforeEach(function () { @@ -44,37 +55,74 @@ function anUpdateIsAvailable(): void $settings->set(Setting::LatestReleasePublishedAt, '2026-08-09T00:00:00Z'); } -test('a container reports itself as one', function () { +test('a container running the published image reports itself as one', function () { expect((new FakeInstallation(container: true))->kind())->toBe(InstallationKind::Container); }); +test('a container built from a checkout is told apart from the image', function () { + // `docker compose pull` on this stack skips every ProjectSend service + // and then reports success, so being handed that command is how an + // installation stays on the version it is on. + expect((new FakeInstallation(container: true, source: true))->kind())->toBe(InstallationKind::ContainerSource); +}); + +test('a working tree is only asked about inside a container', function () { + expect((new FakeInstallation(container: false, source: true))->kind())->toBe(InstallationKind::Manual); +}); + +test('the image says so itself, whatever is on disk', function () { + // Precedence, asserted where it matters: the test suite runs from a + // working tree, so without the marker this same object answers + // ContainerSource. An operator bind-mounting a checkout into the + // published image is still updating it by pulling. + $installation = new class extends Installation + { + protected function inContainer(): bool + { + return true; + } + }; + + expect($installation->kind())->toBe(InstallationKind::ContainerSource); + + putenv('PROJECTSEND_IMAGE=1'); + + try { + expect($installation->kind())->toBe(InstallationKind::Container); + } finally { + putenv('PROJECTSEND_IMAGE'); + } +})->skip(fn () => ! file_exists(base_path('.git')), 'Asserts precedence over a working tree, and there is none here.'); + test('anything else is treated as a manual install', function () { // The safer wrong answer: manual instructions are steps a person reads // and checks, the container command is one they would paste. expect((new FakeInstallation(container: false))->kind())->toBe(InstallationKind::Manual); }); -test('the dashboard tells the frontend which kind of install this is', function (bool $container, string $expected) { - app()->instance(Installation::class, new FakeInstallation($container)); +test('the dashboard tells the frontend which kind of install this is', function (bool $container, bool $source, string $expected) { + app()->instance(Installation::class, new FakeInstallation($container, $source)); $this->actingAs($this->admin) ->get('/dashboard') ->assertInertia(fn (AssertableInertia $page) => $page->where('system.install_kind', $expected)); })->with([ - 'container' => [true, 'container'], - 'manual' => [false, 'manual'], + 'the published image' => [true, false, 'container'], + 'built from a checkout' => [true, true, 'container-source'], + 'manual' => [false, false, 'manual'], ]); -test('the update notice carries the install kind too', function (bool $container, string $expected) { - app()->instance(Installation::class, new FakeInstallation($container)); +test('the update notice carries the install kind too', function (bool $container, bool $source, string $expected) { + app()->instance(Installation::class, new FakeInstallation($container, $source)); anUpdateIsAvailable(); $this->actingAs($this->admin) ->get('/dashboard') ->assertInertia(fn (AssertableInertia $page) => $page->where('update_notice.install_kind', $expected)); })->with([ - 'container' => [true, 'container'], - 'manual' => [false, 'manual'], + 'the published image' => [true, false, 'container'], + 'built from a checkout' => [true, true, 'container-source'], + 'manual' => [false, false, 'manual'], ]); test('no update means no notice, whatever the install kind', function () { diff --git a/tests/Feature/Platform/StaleCodeNoticeTest.php b/tests/Feature/Platform/StaleCodeNoticeTest.php index d4145676..4dac4724 100644 --- a/tests/Feature/Platform/StaleCodeNoticeTest.php +++ b/tests/Feature/Platform/StaleCodeNoticeTest.php @@ -18,12 +18,20 @@ use App\Modules\Platform\Settings\Settings; */ class NoticeInstallation extends Installation { - public function __construct(private readonly bool $container) {} + public function __construct( + private readonly bool $container, + private readonly bool $source = false, + ) {} protected function inContainer(): bool { return $this->container; } + + protected function builtFromSource(): bool + { + return $this->source; + } } function applied(string $version): void @@ -107,14 +115,15 @@ test('it is not gated on the edition', function () { expect(noticeFor($this->admin))->not->toBeNull(); }); -test('it names the command this kind of installation can actually run', function (bool $container, string $expected) { - app()->instance(Installation::class, new NoticeInstallation($container)); +test('it names the command this kind of installation can actually run', function (bool $container, bool $source, string $expected) { + app()->instance(Installation::class, new NoticeInstallation($container, $source)); applied('2.2.0'); expect(noticeFor($this->admin)['install_kind'])->toBe($expected); })->with([ - 'a container' => [true, InstallationKind::Container->value], - 'a server somebody administers' => [false, InstallationKind::Manual->value], + 'a container from the published image' => [true, false, InstallationKind::Container->value], + 'a container built from a checkout' => [true, true, InstallationKind::ContainerSource->value], + 'a server somebody administers' => [false, false, InstallationKind::Manual->value], ]); // The rollback story, asserted end to end: whatever the marker said, the