From bb61c370a06873d8d0a204005ff8ae68de61466b Mon Sep 17 00:00:00 2001 From: Jan-Niclas Loosen Date: Sat, 26 Sep 2026 19:29:29 +0200 Subject: [PATCH] fix(security): honour Pods field restrictions (audit M4) PodsFieldSource maps hidden, read_only, admin_only and restrict_role/-capability like Pods' own view: hidden -> new status HIDDEN, read_only -> READONLY, restricted and not permitted -> READONLY with read_only_restricted, else HIDDEN. HIDDEN fields are not described, read out, queried or written (they look unknown) but stay in the schema for the media usage search; write() re-checks before Pods' save(). Required-but-restricted fields no longer block a create. Pods is not active on DDEV: covered by unit tests with a field-option fixture. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7 --- readme.txt | 3 + src/Integrations/Pods/PodsFieldSource.php | 47 +++++++++++- .../WordPress/Content/PostPresenter.php | 3 +- .../WordPress/Schema/ContentSchema.php | 6 +- .../WordPress/Schema/FieldDefinition.php | 7 ++ .../WordPress/Schema/FieldValidator.php | 6 +- .../Integrations/Pods/PodsFieldSourceTest.php | 74 +++++++++++++++++++ .../WordPress/Content/PostPresenterTest.php | 5 +- .../WordPress/Schema/ContentSchemaTest.php | 6 +- .../WordPress/Schema/FieldValidatorTest.php | 14 ++++ 10 files changed, 163 insertions(+), 8 deletions(-) diff --git a/readme.txt b/readme.txt index 8cc3e42..fe31e9e 100644 --- a/readme.txt +++ b/readme.txt @@ -56,6 +56,9 @@ Einstellungen: Einstellungen → LIT MCP (Verbindung, Zugriff, Tools, Protokoll, Seiten betrifft das praktisch nie (die Antwort warnt dann, dass der Inhalt beim Speichern verändert wurde). * Nach jeder Divi-Änderung wird der statische CSS-Cache von Divi für die Seite geleert. +* Pods-Felder: Die Feldrechte von Pods gelten auch über LIT MCP. „Verborgen“ und für den Benutzer gesperrte + Felder (nur Admins, Rollen- oder Capability-Beschränkung) werden weder angezeigt noch geschrieben; + „schreibgeschützt“ (bzw. gesperrt mit „schreibgeschützt statt verborgen“) sind nur lesbar. * Uploads über LIT MCP: nur Bilder (jpg, png, gif, webp, avif), PDF, MP4, MP3; Größenlimit einstellbar (Standard 5 MB, höchstens 32 MB; Base64-Upload und Download höchstens 10 MB); Import per URL nur von öffentlich erreichbaren Adressen (interne/lokale Adressen werden abgelehnt). diff --git a/src/Integrations/Pods/PodsFieldSource.php b/src/Integrations/Pods/PodsFieldSource.php index fc22685..2004174 100644 --- a/src/Integrations/Pods/PodsFieldSource.php +++ b/src/Integrations/Pods/PodsFieldSource.php @@ -13,6 +13,12 @@ use Throwable; * Pods field adapter (spec §6.1/§6.2): definitions from pods_api()->load_pod(), * values read and written through the Pods API so relationships and caches stay * consistent. Works with Pods 3.x (Pods\Whatsit\Pod / Pods\Whatsit\Field). + * + * Field restrictions (security audit M4): Pods enforces admin_only, restrict_role/-capability, + * read_only and hidden only in its forms – its PHP API (save() with from = 'save') skips them. + * They are mapped like Pods' own view does: hidden → HIDDEN; read_only → READONLY; restricted + * and not permitted for the current user → READONLY with read_only_restricted, else HIDDEN. + * write() refuses every field that is not writable before anything reaches Pods. */ final class PodsFieldSource implements FieldSource { @@ -34,10 +40,20 @@ final class PodsFieldSource implements FieldSource /** @var callable(string, int): mixed */ private $podFor; - public function __construct(?callable $loadPod = null, ?callable $podFor = null) + /** @var callable(object): ?bool Pods field => null (unrestricted) or whether the current user may use it. */ + private $permitted; + + public function __construct(?callable $loadPod = null, ?callable $podFor = null, ?callable $permitted = null) { $this->loadPod = $loadPod ?? static fn (string $name): mixed => pods_api()->load_pod(['name' => $name]); $this->podFor = $podFor ?? static fn (string $name, int $id): mixed => pods($name, $id); + $this->permitted = $permitted ?? static function (object $field): ?bool { + if (! function_exists('pods_has_permissions') || ! pods_has_permissions($field)) { + return null; + } + + return function_exists('pods_permission') && (bool) pods_permission($field); + }; } public static function isAvailable(): bool @@ -93,6 +109,11 @@ final class PodsFieldSource implements FieldSource { $data = []; foreach ($values as $key => $value) { + $status = isset($definitions[$key]) ? $definitions[$key]->status : null; + if ($status === FieldDefinition::READONLY || $status === FieldDefinition::HIDDEN) { + throw new RuntimeException(sprintf('Pods field %s is restricted for the current user.', $key)); + } + $multiple = isset($definitions[$key]) && $definitions[$key]->multiple; $data[$key] = $value ?? ($multiple ? [] : ''); } @@ -140,7 +161,7 @@ final class PodsFieldSource implements FieldSource (string) $field->get_label(), $normalized, 'pods', - $normalized === 'unknown' ? FieldDefinition::UNKNOWN : FieldDefinition::WRITABLE, + $this->status($field, $normalized === 'unknown' ? FieldDefinition::UNKNOWN : FieldDefinition::WRITABLE), (bool) $field->is_required(), (bool) $field->is_multi_value(), $choices, @@ -149,6 +170,28 @@ final class PodsFieldSource implements FieldSource ); } + /** $open: the status without restrictions (writable or unknown). */ + private function status(object $field, string $open): string + { + if (self::flag($field, 'hidden')) { + return FieldDefinition::HIDDEN; + } + + if (($this->permitted)($field) === false) { + return self::flag($field, 'read_only_restricted') ? FieldDefinition::READONLY : FieldDefinition::HIDDEN; + } + + return self::flag($field, 'read_only') ? FieldDefinition::READONLY : $open; + } + + /** A Pods boolean option (stored as 1/'1'/true, like pods_v_bool()). */ + private static function flag(object $field, string $name): bool + { + $value = $field->get_arg($name, false); + + return $value === true || in_array(strtolower(trim((string) $value)), ['1', 'true', 'yes', 'on'], true); + } + /** @return list */ private function choices(string $raw): array { diff --git a/src/Integrations/WordPress/Content/PostPresenter.php b/src/Integrations/WordPress/Content/PostPresenter.php index 5e05f47..643cd48 100644 --- a/src/Integrations/WordPress/Content/PostPresenter.php +++ b/src/Integrations/WordPress/Content/PostPresenter.php @@ -180,6 +180,7 @@ class PostPresenter } /** + * Fields hidden for the current user (FieldDefinition::HIDDEN, e.g. restricted Pods fields), * Pods password fields and readonly protected meta (WordPress' own "_"-prefixed * internal keys) are never read out via lit-content-get/-query: neither belongs in * front of the model, and both are one plugin registration away from leaking secrets @@ -189,7 +190,7 @@ class PostPresenter */ public static function isHiddenField(FieldDefinition $field): bool { - if ($field->type === 'password') { + if ($field->type === 'password' || $field->status === FieldDefinition::HIDDEN) { return true; } diff --git a/src/Integrations/WordPress/Schema/ContentSchema.php b/src/Integrations/WordPress/Schema/ContentSchema.php index 506e49f..d4331df 100644 --- a/src/Integrations/WordPress/Schema/ContentSchema.php +++ b/src/Integrations/WordPress/Schema/ContentSchema.php @@ -93,9 +93,13 @@ class ContentSchema ]; if ($withFields) { + $visible = array_filter( + $this->fields($postType), + static fn (FieldDefinition $field): bool => $field->status !== FieldDefinition::HIDDEN + ); $description['fields'] = array_values(array_map( static fn (FieldDefinition $field): array => $field->toArray(), - $this->fields($postType) + $visible )); } diff --git a/src/Integrations/WordPress/Schema/FieldDefinition.php b/src/Integrations/WordPress/Schema/FieldDefinition.php index 87bbfbb..91aab4b 100644 --- a/src/Integrations/WordPress/Schema/FieldDefinition.php +++ b/src/Integrations/WordPress/Schema/FieldDefinition.php @@ -13,6 +13,13 @@ final class FieldDefinition public const READONLY = 'readonly'; public const UNKNOWN = 'unknown'; + /** + * Not for the current user (e.g. a Pods field restricted to admins or marked hidden, security + * audit M4): never read out, described or written – it looks like an unknown field. Kept in + * the schema so the media usage search still sees attachment IDs stored in it. + */ + public const HIDDEN = 'hidden'; + /** * @param list|null $choices * @param array{object_type: string, name: ?string}|null $related diff --git a/src/Integrations/WordPress/Schema/FieldValidator.php b/src/Integrations/WordPress/Schema/FieldValidator.php index 7bf3926..7bdea47 100644 --- a/src/Integrations/WordPress/Schema/FieldValidator.php +++ b/src/Integrations/WordPress/Schema/FieldValidator.php @@ -27,7 +27,7 @@ final class FieldValidator foreach ($values as $key => $value) { $key = (string) $key; $field = $definitions[$key] ?? null; - if ($field === null) { + if ($field === null || $field->status === FieldDefinition::HIDDEN) { $errors[$key] = 'Unbekanntes Feld; verfügbare Felder über lit/content-types (direkt: ' . 'lit-content-types) abrufen.'; continue; @@ -55,7 +55,9 @@ final class FieldValidator } foreach ($definitions as $key => $field) { - if (! $field->required || isset($errors[$key])) { + // A field the user cannot write (readonly, hidden) cannot be required from them either. + $writable = in_array($field->status, [FieldDefinition::WRITABLE, FieldDefinition::UNKNOWN], true); + if (! $field->required || ! $writable || isset($errors[$key])) { continue; } diff --git a/tests/Unit/Integrations/Pods/PodsFieldSourceTest.php b/tests/Unit/Integrations/Pods/PodsFieldSourceTest.php index fe6d779..e78c181 100644 --- a/tests/Unit/Integrations/Pods/PodsFieldSourceTest.php +++ b/tests/Unit/Integrations/Pods/PodsFieldSourceTest.php @@ -141,4 +141,78 @@ final class PodsFieldSourceTest extends TestCase (new PodsFieldSource(null, static fn () => $pod))->write(5, 'lit_person', ['role' => 'x'], []); } + + /** + * Security audit M4: Pods field restrictions, as Pods' own view applies them (Pods::view): + * hidden → hidden; read_only → readonly; restricted (admin_only, restrict_role/-capability) + * and not permitted → readonly with read_only_restricted, else hidden. + * + * @param array $permitted Field name => pods_permission() (null = unrestricted). + * @return array Field name => status. + */ + private function statuses(array $permitted): array + { + $pod = Mockery::mock(); + $pod->allows('get_type')->andReturn('post_type'); + $pod->allows('get_fields')->andReturn([ + $this->field('open', 'text'), + $this->field('internal', 'text', ['hidden' => '1']), + $this->field('fixed', 'text', ['read_only' => 1]), + $this->field('boss', 'text', ['admin_only' => '1']), + $this->field('team', 'text', ['restrict_role' => '1', 'read_only_restricted' => '1']), + $this->field('cap', 'number', ['restrict_capability' => '1']), + ]); + $source = new PodsFieldSource( + static fn (string $name) => $pod, + null, + static fn (object $field): ?bool => $permitted[$field->get_name()] ?? null + ); + + $statuses = []; + foreach ($source->fieldsFor('lit_person') as $definition) { + $statuses[$definition->key] = $definition->status; + } + + return $statuses; + } + + public function testFieldRestrictionsBecomeReadonlyOrHidden(): void + { + self::assertSame( + [ + 'open' => FieldDefinition::WRITABLE, + 'internal' => FieldDefinition::HIDDEN, + 'fixed' => FieldDefinition::READONLY, + 'boss' => FieldDefinition::HIDDEN, + 'team' => FieldDefinition::READONLY, + 'cap' => FieldDefinition::HIDDEN, + ], + $this->statuses(['boss' => false, 'team' => false, 'cap' => false]) + ); + self::assertSame( + ['open', 'internal', 'fixed', 'boss', 'team', 'cap'], + array_keys($this->statuses([])), + 'hidden fields stay in the schema (media usage search), only their status changes' + ); + $allowed = $this->statuses(['boss' => true, 'team' => true, 'cap' => true]); + self::assertSame(FieldDefinition::WRITABLE, $allowed['boss']); + self::assertSame(FieldDefinition::WRITABLE, $allowed['team']); + self::assertSame(FieldDefinition::HIDDEN, $allowed['internal'], 'hidden is hidden for everyone, as in Pods'); + } + + /** Re-check in write(): a restricted field is never handed to Pods' save(), which skips its own checks. */ + public function testWriteRefusesRestrictedFields(): void + { + $pod = Mockery::mock(); + $pod->expects('save')->never(); + $definitions = [ + 'role' => new FieldDefinition('role', 'Funktion', 'string', 'pods', FieldDefinition::WRITABLE), + 'boss' => new FieldDefinition('boss', 'Boss', 'string', 'pods', FieldDefinition::HIDDEN), + ]; + + $this->expectException(RuntimeException::class); + + (new PodsFieldSource(null, static fn () => $pod)) + ->write(5, 'lit_person', ['role' => 'x', 'boss' => 'y'], $definitions); + } } diff --git a/tests/Unit/Integrations/WordPress/Content/PostPresenterTest.php b/tests/Unit/Integrations/WordPress/Content/PostPresenterTest.php index 8c1fed3..64c063b 100644 --- a/tests/Unit/Integrations/WordPress/Content/PostPresenterTest.php +++ b/tests/Unit/Integrations/WordPress/Content/PostPresenterTest.php @@ -113,11 +113,14 @@ final class PostPresenterTest extends TestCase $password = new FieldDefinition('secret', 'Secret', 'password', 'pods', 'writable'); $protected = new FieldDefinition('_internal', 'Internal', 'string', 'meta', 'readonly'); $visible = new FieldDefinition('role', 'Funktion', 'string', 'pods', 'writable'); + $restricted = new FieldDefinition('boss', 'Boss', 'string', 'pods', FieldDefinition::HIDDEN); $source = Mockery::mock(FieldSource::class); $source->expects('read')->once()->with(5, $visible)->andReturn('Leitung'); $schema = Mockery::mock(ContentSchema::class); $schema->allows('taxonomies')->andReturn([]); - $schema->allows('fields')->andReturn(['secret' => $password, '_internal' => $protected, 'role' => $visible]); + $schema->allows('fields')->andReturn([ + 'secret' => $password, '_internal' => $protected, 'role' => $visible, 'boss' => $restricted, + ]); $schema->allows('sourceOf')->andReturn($source); $post = new WP_Post(['ID' => 5, 'post_type' => 'lit_person']); diff --git a/tests/Unit/Integrations/WordPress/Schema/ContentSchemaTest.php b/tests/Unit/Integrations/WordPress/Schema/ContentSchemaTest.php index 00fd25f..8a780c2 100644 --- a/tests/Unit/Integrations/WordPress/Schema/ContentSchemaTest.php +++ b/tests/Unit/Integrations/WordPress/Schema/ContentSchemaTest.php @@ -76,7 +76,10 @@ final class ContentSchemaTest extends TestCase (object) ['name' => 'post_format', 'show_ui' => false, 'hierarchical' => false, 'labels' => (object) ['name' => 'Formate']], ]); $schema = new ContentSchema( - [$this->source('meta', [new FieldDefinition('footnotes', 'footnotes', 'string', 'meta', 'writable')])], + [$this->source('meta', [ + new FieldDefinition('footnotes', 'footnotes', 'string', 'meta', 'writable'), + new FieldDefinition('zz_hidden', 'Verborgen', 'string', 'pods', FieldDefinition::HIDDEN), + ])], static fn (string $type): bool => $type === 'page' ); @@ -92,6 +95,7 @@ final class ContentSchemaTest extends TestCase self::assertSame([['name' => 'category', 'label' => 'Kategorien', 'hierarchical' => true]], $withoutFields['taxonomies']); self::assertArrayNotHasKey('fields', $withoutFields); self::assertSame('footnotes', $withFields['fields'][0]['key']); + self::assertCount(1, $withFields['fields'], 'hidden (restricted) fields are not described (M4)'); self::assertNull($schema->describe('et_pb_layout', false)); } diff --git a/tests/Unit/Integrations/WordPress/Schema/FieldValidatorTest.php b/tests/Unit/Integrations/WordPress/Schema/FieldValidatorTest.php index 4887f32..f739cbc 100644 --- a/tests/Unit/Integrations/WordPress/Schema/FieldValidatorTest.php +++ b/tests/Unit/Integrations/WordPress/Schema/FieldValidatorTest.php @@ -32,6 +32,7 @@ final class FieldValidatorTest extends TestCase 'secret' => new FieldDefinition('_secret', '_secret', 'string', 'meta', 'readonly'), 'city' => new FieldDefinition('city', 'city', 'unknown', 'sample', 'unknown'), 'pin' => new FieldDefinition('pin', 'PIN', 'password', 'pods', 'writable'), + 'boss' => new FieldDefinition('boss', 'Boss', 'string', 'pods', FieldDefinition::HIDDEN), ]; } @@ -72,8 +73,10 @@ final class FieldValidatorTest extends TestCase 'mentor' => 7, 'photos' => 3, 'start' => '01.10.2026', + 'boss' => 'x', ], false)['errors']; + self::assertSame($errors['nope'], $errors['boss'], 'a hidden field looks like an unknown one (M4)'); self::assertStringContainsString('lit-content-types', $errors['nope']); self::assertSame('Feld ist schreibgeschützt.', $errors['secret']); self::assertSame('Erlaubte Werte: junior, senior.', $errors['level']); @@ -123,4 +126,15 @@ final class FieldValidatorTest extends TestCase self::assertCount(1, $result['warnings']); self::assertStringContainsString('city', $result['warnings'][0]); } + + /** M4: a required field the user cannot write (restricted Pods field) cannot block a create. */ + public function testRequiredButRestrictedFieldsAreNotDemanded(): void + { + $definitions = [ + 'boss' => new FieldDefinition('boss', 'Boss', 'string', 'pods', FieldDefinition::HIDDEN, true), + 'fixed' => new FieldDefinition('fixed', 'Fest', 'string', 'pods', FieldDefinition::READONLY, true), + ]; + + self::assertSame([], (new FieldValidator())->validate($definitions, [], true)['errors']); + } }