feat(post-form): U2 — server-side active-trip + owner scope guard on edit save

Enforce R6 on the save path (KTD6): in cache-on-save's onFormValidationProcessed, when a hidden edit_path is present, require the site owner (not merely any login — the super-admin tester also authenticates) AND that the target resolves through the page tree to a direct child of the active trip's dailies container. Fail closed with a ValidationException so add_page never runs. Create (empty edit_path) is left untouched. New shared EntryScopeGuard (classes/EntryScopeGuard.php) is the single source of truth for both R6 enforcement points — this save guard and U6's delete route call the same isOwner()/resolveActiveDailyChild()/segment helpers, so they cannot diverge (KTD5). Resolution is via $pages->find() + a parent-route assertion, never raw path concatenation, closing the traversal hole (basename(dirname()) yields the same target add-page-by-form writes to). Verified on the 2.0.4 container: non-owner edit, out-of-scope edit_path, unsafe '..' segment, and non-dailies-child targets are all rejected; owner in-place edit succeeds (V3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1FrCYNq6RXdGYbn5PFrhM
2026-07-04 23:40:04 +02:00
parent aa5d34717e
commit 4e91492f65
2 changed files with 138 additions and 7 deletions
@@ -5,6 +5,10 @@ use Grav\Common\Data\ValidationException;
 use Grav\Common\Plugin;
 use RocketTheme\Toolbox\Event\Event;

+require_once __DIR__ . '/classes/EntryScopeGuard.php';
+
+use Grav\Plugin\Shared\EntryScopeGuard;
+
 class CacheOnSavePlugin extends Plugin
 {
    public static function getSubscribedEvents(): array
@@ -51,14 +55,31 @@ class CacheOnSavePlugin extends Plugin

        // One shared /post form drives both create and edit (KTD1). add-page-by-form
        // reads overwrite_mode from the /post page header's pageconfig (not form
-        // data), so we toggle it here per submit:
-        //  - edit  (edit_path present) -> overwrite_mode:edit, writes back in place
-        //  - create (edit_path empty)  -> overwrite_mode:false, so stock falls
-        //    through to slug_field (date,title) and writes a fresh dated folder.
-        // This keeps add-page-by-form stock (no fork), since a static
-        // overwrite_mode:edit would break create (empty edit_path slugs to '.').
+        // data), so we toggle it here per submit.
        $editPath = $this->editPathFromForm($form);
-        $this->setOverwriteMode($editPath === '' ? 'false' : 'edit');
+
+        if ($editPath === '') {
+            // CREATE — left untouched (any site.login user): overwrite_mode:false
+            // so stock add-page-by-form falls through to slug_field (date,title)
+            // and writes a fresh dated folder.
+            $this->setOverwriteMode('false');
+            return;
+        }
+
+        // EDIT — enforce R6 server-side (KTD6) BEFORE allowing an in-place write.
+        // Fail closed (ValidationException) so the add_page action never runs.
+        // The UI only renders Edit for the owner on the active trip, but that gate
+        // is cosmetic; this is the authoritative check.
+        if (!EntryScopeGuard::isOwner($this->grav)) {
+            throw new ValidationException('You are not allowed to edit journal entries.');
+        }
+        $segment = EntryScopeGuard::segmentFromEditPath($editPath);
+        if (EntryScopeGuard::resolveActiveDailyChild($this->grav, $segment) === null) {
+            // Unsafe/traversal segment, no active trip, missing page, or a target
+            // outside the active trip's dailies — all rejected identically.
+            throw new ValidationException('That entry is not editable here — it is not in the active trip.');
+        }
+        $this->setOverwriteMode('edit');
    }

    /**
@@ -0,0 +1,110 @@
+<?php
+namespace Grav\Plugin\Shared;
+
+use Grav\Common\Grav;
+use Grav\Common\Page\Interfaces\PageInterface;
+
+/**
+ * Single source of truth for the R6 server-side scope guard, shared by BOTH
+ * enforcement points so they cannot diverge (KTD5):
+ *   - the save/publish path (cache-on-save onFormValidationProcessed), and
+ *   - the delete path (entry-actions API route).
+ *
+ * Two independent checks, both required:
+ *   1. isOwner()  — the acting user is the configured site owner, NOT merely any
+ *      authenticated account (the super-admin `tester` also authenticates).
+ *   2. resolveActiveDailyChild() — the target resolves, through the page tree,
+ *      to a real DIRECT child of the active trip's `dailies` container. Resolving
+ *      via $pages->find() + a parent-route assertion (never raw path concatenation)
+ *      closes the traversal hole where a string like `/…/dailies/../other/entry.md`
+ *      prefix-matches the active dailies but points elsewhere.
+ */
+class EntryScopeGuard
+{
+    /**
+     * Active trip's dailies container route ("/trips/<slug>/dailies"), or null
+     * when site.active_trip is unset. Accepts a full route or a bare slug.
+     */
+    public static function dailiesRoute(Grav $grav): ?string
+    {
+        $active = $grav['config']->get('site.active_trip');
+        $active = is_string($active) ? trim($active) : '';
+        if ($active === '') {
+            return null;
+        }
+        $trip = trim($active, '/');
+        if (strpos($trip, 'trips/') !== 0) {
+            $trip = 'trips/' . $trip;
+        }
+        return '/' . $trip . '/dailies';
+    }
+
+    /**
+     * True only when the current user is authenticated AND their username equals
+     * site.owner_username. Gating on authentication alone would grant rights to
+     * every account, including the super-admin `tester` (KTD8).
+     */
+    public static function isOwner(Grav $grav): bool
+    {
+        $user = $grav['user'] ?? null;
+        if (!$user || empty($user->authenticated)) {
+            return false;
+        }
+        $owner = $grav['config']->get('site.owner_username');
+        return is_string($owner) && $owner !== '' && $user->username === $owner;
+    }
+
+    /**
+     * A safe single path segment: non-empty, no separators, no dot-traversal.
+     */
+    public static function isSafeSegment(string $segment): bool
+    {
+        if ($segment === '' || $segment === '.' || $segment === '..') {
+            return false;
+        }
+        if (strpbrk($segment, '/\\') !== false) {
+            return false;
+        }
+        return strpos($segment, '..') === false;
+    }
+
+    /**
+     * The folder segment carried by a hidden edit_path value. post-form.js sets
+     * edit_path to "<entry-route>/entry.md", so basename(dirname()) is the entry's
+     * own folder name (its route's last segment) — the same value stock
+     * add-page-by-form derives for the in-place write.
+     */
+    public static function segmentFromEditPath(string $editPath): string
+    {
+        $editPath = trim($editPath);
+        if ($editPath === '') {
+            return '';
+        }
+        return basename(dirname($editPath));
+    }
+
+    /**
+     * Resolve a folder segment to the page that is a DIRECT child of the active
+     * trip's dailies container, or null when the segment is unsafe, no active trip
+     * is set, the page does not exist, or its parent is not the active dailies.
+     */
+    public static function resolveActiveDailyChild(Grav $grav, string $segment): ?PageInterface
+    {
+        if (!self::isSafeSegment($segment)) {
+            return null;
+        }
+        $dailies = self::dailiesRoute($grav);
+        if ($dailies === null) {
+            return null;
+        }
+        $page = $grav['pages']->find($dailies . '/' . $segment);
+        if ($page === null) {
+            return null;
+        }
+        $parent = $page->parent();
+        if ($parent === null || $parent->route() !== $dailies) {
+            return null;
+        }
+        return $page;
+    }
+}