WP-LIT-MCP/docs/superpowers/notes/2026-09-26-etappe-2b-security-audit.md

# WP-LIT-MCP: whole-plugin security audit (S42 step 2)

- **Scope:** branch etappe-2, HEAD 314b3d0.
- **Environment:** WordPress 7.1.2, Divi 5.2.1 on DDEV.
- **Method:**
  - Read all of `src/`, plus the relevant parts of `vendor/wp-media/mcp-oauth` and `vendor/wordpress/mcp-adapter`.
  - Ran read-only DDEV `wp eval-file` probes.
  - Ran one live, self-cleaning run of the real `lit/content-create` ability as a temporary Author:
    - user 67 and post 861 were created, then deleted;
    - the deletion was verified afterwards: no `lit_audit_*` user and no probe post remain.
  - No code was changed. The option `lit_mcp_allowed_roles` was widened in-process only, through the filter, never in the database.

No Critical or High findings. The Divi value pipeline itself held.
The two most relevant problems are ways *around* it:
- the raw `content` field;
- the tool-group switch.

---

## Findings (most severe first)

### M1 – Medium: raw `content` lets users without unfiltered_html write the "builder-only" Divi values (the S7/S26 gates don't apply)

**Where**
- `src/Integrations/WordPress/Content/PostWriter.php:28` (content → post_content) and `:58`.
- `ContentCreateAbility.php:127-133`.
- `ContentUpdateAbility.php:124` refuses `content` only when the post *already* uses Divi. Nothing refuses content that *introduces* Divi markup.
- Compare `src/Integrations/Divi/Compose/ValueCleaner.php:57` (`BUILDER_ONLY`).

**Scenario**
1. An Author, or an Editor on multisite or with `DISALLOW_UNFILTERED_HTML`, calls `lit/content-create` (or `lit/content-update` on a non-Divi post).
2. The `content` is Divi 5 block markup that sets:
   - a module's `css.desktop.value.freeForm` (e.g. `body{…}`);
   - `module.decoration.attributes` (custom HTML attributes).
3. kses (`filter_block_content`) and Divi's own `wp_insert_post_data` security filters keep both. Only `on*` attributes are stripped, by Divi.
4. The page is now a Divi layout. From then on every `lit-divi-*` tool works on it inside the kses window.

The same values are refused with "nur im Divi-Builder" (only in the Divi builder) when sent through `lit-divi-insert`/`-update` (rulings S7/S26).

A prompt-injected Claude session can therefore do CSS injection or attribute injection on the live site. This is exactly what S26 was meant to prevent.

**Evidence**
- Live run of `wp_get_ability('lit/content-create')->execute()` as a temporary Author (unfiltered_html=false):
  - `freeForm CSS stored: true`;
  - `data-probe stored: true`;
  - rendered `<div class="et_pb_text_0 … et_pb_module et_flex_module" data-probe="1">`.
- Read-only kses probe (`kses6.php`): `onmouseover` is removed by Divi and never rendered. No script execution found. Divi renders a `javascript:` `linkUrl` as `href=""`.

**Parity note**
The same user can store the same markup through core REST (`/wp/v2/posts`) or the code editor. So this is no privilege escalation beyond WordPress core. It is a bypass of the plugin's own, explicitly decided gate. The pipeline guarantee "every path from ability input to post_content goes through ValueCleaner/Composer/TreeEditor" does not hold for `content`.

**Fix proposal**
- In `PostWriter::prepare()` (create and update), refuse a `content` that contains `<!-- wp:divi/` (and `[et_pb_`), with a hint to use `divi` / the Divi tools.
- Alternative: parse it with `Parser` and run every Divi block through `Composer`/`ValueCleaner` (heavier).
- At minimum, do this for users without `unfiltered_html`.

### M2 – Medium: "nur lesen" / disabled Divi group does not stop Divi writes (content-create `divi`)

**Where**
- `src/Integrations/WordPress/WordPressIntegration.php:83` passes `$this->divi` (the ContentComposer) unconditionally.
- `ContentCreateAbility.php:113-119, 145` write the Divi layout whenever Divi 5 is loaded.
- `ExposurePolicy::disabledByReadonly()` and `enabled()` only look at the ability's own group ("wordpress").

**Scenario**
1. The admin sets group Divi to "nur lesen", or disables `lit-divi-insert`, or disables the whole group.
2. The tools tab, readme ("schaltet alle schreibenden Tools einer Gruppe … ab", switches off all writing tools of a group) and acceptance note §7 all promise that no Divi writes happen.
3. Claude can still create complete Divi layouts via `lit-content-create` with `divi: [...]`. That goes through the same kses window.

Related cross-group writes that still work with Divi "nur lesen":
- `lit/post-duplicate`, `lit/post-replace-with-copy`, `lit/revision-restore` on Divi pages. These use stored content, so the impact is lower.

**Evidence:** by reading. The wiring is unconditional; ExposurePolicy has no cross-group notion.

**Fix proposal**
- Give `ContentCreateAbility` a callable `diviAllowed()` that is true only when `ExposurePolicy::mode()` of `lit/divi-insert` is not `disabled`. That mode already includes the Divi group's read-only switch.
- If Divi writes are not allowed, answer `divi` with `unsupported`, and drop the `divi` property from the input schema.
- Document in the Tools tab that duplicate/replace/restore copy stored Divi content regardless of the Divi group.

### M3 – Medium: URL import SSRF – DNS rebinding TOCTOU, core-version-dependent blocklist, error oracle

**Where**
- `src/Integrations/Media/MediaImporter.php:55-62` (`wp_http_validate_url` pre-check).
- `:119-139` (`wp_safe_remote_get` + `CURLOPT_IPRESOLVE_V4`).
- `:142` / `:339-346` (the raw `WP_Error` message is returned).

**Scenario 1: DNS rebinding**
- A prompt-injected Claude (e.g. a web page Claude read says "import this image") calls `lit-media-upload-from-url` with a hostname.
- `wp_http_validate_url()` resolves the host with `gethostbyname()` and sees a public IP. cURL then resolves again; with TTL 0 it gets 127.0.0.1 or another internal host.
- Redirects are re-validated by core (`WP_Http::validate_redirects`, verified in 7.1.2), but each redirect has the same TOCTOU gap.
- Impact:
  - blind GET requests to internal services on ports 80/443/8080;
  - internal *images/PDFs* (e.g. internal dashboards rendering PNG) get copied into the public media library.

**Scenario 2: version-dependent blocklist**
- On DDEV (core 7.1.2), `wp_http_validate_url()` blocks 169.254/16, 100.64/10, TEST-NETs, multicast etc. Verified: `ssrf1.php`, all 17 internal probes return false.
- The plugin declares "Requires at least: 6.9". Older cores only block 0/8, 10/8, 127/8, 172.16/12 and 192.168/16.
- If 6.9 lacks the newer ranges, `http://169.254.169.254/` (cloud metadata) is reachable. Not verified on 6.9.

**Scenario 3: error oracle**
- `Download fehlgeschlagen: <cURL/WP message>` and `HTTP-Status %d` let the caller tell apart:
  - refused;
  - timeout;
  - 404;
  - "wrong file type".

**Evidence:** by reading, plus the DDEV validation probe.

**Fix proposal**
- In the existing `http_api_curl` hook:
  - resolve the host once;
  - check the IP with the plugin's own full private/reserved list (`filter_var(..., FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE)`, plus 100.64/10);
  - pin it with `CURLOPT_RESOLVE` (`host:port:ip`).
- Set `'redirection' => 0` and follow at most 3 redirects manually, applying the same resolve, check and pin to each.
- Return a generic "Download fehlgeschlagen" message and log the detail.

### M4 – Medium (only when Pods field restrictions are used): Pods field access restrictions are ignored

**Where**
- `src/Integrations/Pods/PodsFieldSource.php:73-114` (read/write through `pods()->field()` and `pods()->save()`).
- `PostWriter.php:226` (`META_SOURCES` excludes pods, so there is no per-field capability check).
- `PostPresenter::isHiddenField()` only hides the `password` type.

**Scenario**
- Pods fields can be restricted per field:
  - `admin_only`;
  - `restrict_role`;
  - `restrict_capability`;
  - `read_only`;
  - `hidden`.
- Pods enforces these only in its forms (`PodsForm::permission` → `pods_permission`).
- Its PHP API does not: `PodsAPI::save_pod_item` defaults `from = 'save'`, and `'save' === $params->from` skips `pods_permission` (pods/classes/PodsAPI.php:4868, 4994).
- An Editor or Author with `edit_post` on the entry can therefore read (`lit-content-get`, `lit-content-query` field filter) and write (`lit-content-update/create`) fields the site owner restricted to admins or made read-only.

**Evidence:** by reading the plugin and the Pods source in `wp-content/plugins/pods`. Pods is not active on DDEV, so there was no live test.

**Fix proposal**
- In `PodsFieldSource::definition()`:
  - mark fields `READONLY` when `pods_has_permissions($field) && ! pods_permission($field)` or when `read_only` is set;
  - hide them (as `isHiddenField` does) when `hidden` is set or permission fails without `read_only_restricted`.
- Re-check in `write()` before `save()`.

### L1 – Low: any logged-in user can complete OAuth and get a connection (application password + tokens)

**Where**
- The vendor flow (`mcp-oauth` `AuthorizeEndpoint`/`ConsentEndpoint`: only `is_user_logged_in()`).
- The plugin adds no gate: `Plugin.php:110` boots it as-is.
- The `lit_mcp_use` gate only applies at tool execution (`AbilityRegistrar.php:56-63`) and tool listing (`VisibilityFilter`).

**Scenario**
- On sites with customer or subscriber accounts (WooCommerce, memberships), any customer can connect Claude.
- mcp-oauth then creates an application password for them, even where core would not offer app passwords to that user.
- It also issues access and refresh tokens, and mcp-adapter sessions in user meta.
- The customers can't run tools. But the connections appear in "Zugriff", and the site accumulates credentials and sessions for users who were never meant to have them.

**Evidence:** by reading.

**Fix proposal:** before mcp-oauth's authorize/consent handler, refuse with a clear message when `! current_user_can('lit_mcp_use')`. A hook such as `template_redirect` on its rewrite query var would work, and so would a filter if mcp-oauth offers one.

### L2 – Low: release/build hygiene – dev vendor, tests namespace, direct file access

**Where**
- `vendor/` (dev install).
- `composer.json` `autoload-dev`.
- All `src/**/*.php` (no `ABSPATH` guard).
- `tests/Integration/divi-convert-fixture.php` (the only integration script without a `WP_CLI` guard).
- `.gitattributes` (only relevant if the zip is made with `git archive`, and `vendor/` is git-ignored).

**Scenario and evidence**

1. **Dev vendor loaded at runtime.** The current tree's Jetpack autoloader loads these files on every WP request:
   - `mockery/library/helpers.php`;
   - `Mockery.php`;
   - `brain/monkey/inc/api.php`;
   - `phpunit/.../Assert/Functions.php`.

   Source: `vendor/composer/jetpack_autoload_filemap.php`. No release script exists (spec §U is still open).

2. **Tests namespace survives `--no-dev`.** Reproduced: `composer install --no-dev` in a temp dir still gives `LIT\MCP\Tests\ → tests/` in `vendor/composer/jetpack_autoload_psr4.php:21`. The Jetpack autoloader keeps root `autoload-dev`. If `tests/` ships, test classes are autoloadable in production.

3. **Direct HTTP access leaks paths and versions.**
   - `curl https://…/wp-content/plugins/WP-LIT-MCP/src/Admin/AccessTab.php` returns a PHP fatal error with the full server path (display_errors on in DDEV).
   - `…/tests/Integration/divi-convert-fixture.php` → HTTP 200.
   - `…/vendor/composer/installed.json` → HTTP 200 (full dependency and version list).

**Fix proposal**
- Add a build script:
  - `composer install --no-dev -o`;
  - copy only `wp-lit-mcp.php`, `uninstall.php`, `readme.txt`, `src/`, `assets/`, `docs/guides/`, `vendor/`;
  - no `tests/`, `docs/superpowers`, `.superpowers`, `composer.*`, `phpunit/phpcs` configs.
- Add a CI check that the zip contains no `vendor/phpunit`, `vendor/mockery` or `tests/`.
- Add `defined('ABSPATH') || exit;` to the src files (Plugin Check flags its absence), and the `WP_CLI` guard to `divi-convert-fixture.php`.

### L3 – Low: `tree_rev` / `rev` lock is check-then-write (lost update)

**Where**
- `DiviWriter.php:116` (check) → `:161` (save).
- `ContentUpdateAbility.php:115` → `:156`.
- `MediaUpdateAbility` `modified`.

**Scenario:** two parallel writes on the same page (two Claude sessions, or Claude plus the Visual Builder) both pass the rev check. The later `wp_update_post()` silently overwrites the earlier change. The restore-point revision limits the damage.

**Evidence:** by reading. There is no lock or conditional update.

**Fix proposal:**
- Take a short per-post lock around check and save (`wp_cache_add`/`add_option('lit_mcp_lock_<id>')` with expiry, or MySQL `GET_LOCK`).
- Re-read `post_content` inside the lock and compare `Rev` again right before `wp_update_post()`.

### L4 – Low: deactivation and uninstall leave OAuth sessions valid

**Where**
- `Lifecycle.php:93-101`.
- `Uninstaller.php:28-61`. This is documented as intentional.

**Scenario**
- Deactivating or deleting the plugin keeps these:
  - the mcp-oauth application passwords;
  - `mcp_refresh_jti_*` meta;
  - `mcp_jwt_secret`.
- Reactivating it within 30 days revives every refresh token. The admin may believe that removing the plugin cut Claude off.
- Plugin-owned post meta `_lit_mcp_copy_of` / `_lit_mcp_backup_of` also remains.

**Evidence:** by reading.

**Fix proposal:**
- On uninstall, delete the connections `ConnectionRepository` identifies (marker meta plus https `app_id`).
- Either delete `_lit_mcp_*` meta, or mention in readme and deactivation notice that connections survive.
- Offer "alle Verbindungen widerrufen" (revoke all connections) in the Access tab.

### I1 – Info: mcp-oauth logs MCP request bodies when `WP_DEBUG` and `WP_DEBUG_LOG` are on

**Where:** `vendor/wp-media/mcp-oauth/inc/Transport/OAuthHttpTransport.php:128-137` (`McpLogger::safe_request_body()`, 8 KB).

Tool arguments (page content, private drafts, Base64 uploads) go to `wp-content/debug.log`, which is often web-reachable on customer staging sites.

**Fix proposal:** mention this in the readme, or turn it off (e.g. filter or constant, if mcp-oauth offers one; otherwise ask upstream).

### I2 – Info: non-Divi writes re-kses Divi pages for users without unfiltered_html

These tools save through plain `wp_update_post`/`wp_insert_post`, so `content_save_pre` kses re-serializes the whole Divi layout:
- `lit/content-update` (title/excerpt/fields);
- publish/unpublish/schedule;
- `revision-restore`;
- `post-duplicate`;
- `post-replace-with-copy`.

The byte-identity promise of spec §1.3/§5.2 does not hold for these tools. There is no security impact: it only ever sanitizes more.

**Evidence:** read-only probe `kses2.php`. All 7 stored Divi pages changed under editor-kses (e.g. an empty block becomes `/-->`).

### I3 – Info: smaller observations

1. **Multisite super admin bypasses the role gate.** A super admin always has `lit_mcp_use` (`WP_User::has_cap`), even if "Zugriff" allows no role. `AccessTab::currentUserLosesAccess` knows this; the UI text doesn't say it.
2. **Media usage lists unreadable posts.** `MediaUsageFinder.php:455-461` returns post ID and post type (not the title) for posts the user can't read. This lets a user learn that private posts exist. It is intentional, to block deletes.
3. **All post types with a UI are exposed.** Every `show_ui` post type, including non-public ones (on DDEV: `path`), is exposed; only core, Divi internals and `lit_mcp_excluded_post_types` are excluded. Consider documenting the filter for data-protection purposes (form submissions, personal data as CPTs).
4. **Nested REST requests can block later calls.** A `rest_do_request()` to another route during a tool call makes `RequestContext` false for the rest of that HTTP request (`RequestContext.php:281`). This fails closed, so later JSON-RPC calls in the same batch get `forbidden`. Functional only.
5. **`confirm: true` is not a human confirmation.** Claude sets it itself; under prompt injection it is no safeguard. Keep the destructive tools indirect by default.

---

## Checked and found sound

- **RequestContext:** recomputed on every matched REST request, cleared after our route, and required by every ability. WP-CLI/STDIO and other routes fail closed.
- **Default mcp-adapter server:** disabled; the only MCP route is `/mcp/mcp-oauth-server` (verified via `rest_get_server()->get_routes()`).
- **Core Abilities REST:** our abilities have `show_in_rest=false` (only the 3 `core/*` abilities are in REST).
- **Transport and JWT:** `OAuthHttpTransport` requires a Bearer JWT (HS256 pinned, exp, aud = our route, iss = `home_url()`, app password must still exist). The current user is set only from the JWT; cookie or Basic auth can't reach the tools.
- **Revocation:**
  - AccessTab revoke deletes the app password, which the next request's check honours;
  - refresh-token rotation and reuse detection live in mcp-oauth;
  - revoke is limited to `edit_user`-able users and, on multisite, to site members.
- **ConnectionUsage:** records only after a <400 response on our route, and only for `sub === current user`.
- **`lit_mcp_use` gate:** dynamic cap via `user_has_cap` (no role DB changes); the option is sanitized to existing roles; the filter output is type-checked; the meta-tools' capability filters are set to `lit_mcp_use`.
- **Exposure:**
  - disabled and "nur lesen" abilities are never registered, so `execute-ability` answers "not found";
  - DIRECT_ONLY abilities are `mcp.public=false`, so `execute-ability` refuses them;
  - `lit/guide` is always direct.
- **Annotations:** every tool annotated `readonly` really doesn't write (render-summary runs `the_content` side effects only).
- **Per-object checks:**
  - `PostGuard::editable` (`edit_post`, managed type, trash rules) on every content, status, revision, render and Divi ability, in both permission and execute;
  - revisions are checked via the parent and `post_type === 'revision'`;
  - attachments via `MediaGuard` (`edit_post`/`delete_post`, type check);
  - menu items via item ID membership in the menu.
- **Nested IDs:**
  - featured image, parent and relation fields: `read_post` plus type checks;
  - menu `object_id`: `read_post`, no private, unpublished only with a flag, taxonomy must be viewable;
  - Divi media/gallery and `select_post`: `read_post`;
  - terms: `assign_terms`/`edit_terms`;
  - meta: `edit_post_meta`/`add_post_meta` per key.
- **Query and search:**
  - `content-query`/`media-search`/`replace-text` limit the query to the user's own posts without `edit_others_posts` and gate statuses;
  - hidden fields can't be used as a query filter.
- **Settings:**
  - `manage_options`;
  - strict allow-list;
  - `admin_email`/`users_can_register`/`default_role`/`siteurl`/`home` explicitly blocked;
  - per-key validation (installed languages, valid timezones, published pages, image attachment).
- **Meta writes:**
  - `update_post_meta` double-serializes a serialized string, so there is no object injection;
  - protected keys are refused through the meta caps;
  - clone and replace copy raw stored meta via `$wpdb` (no unserialize).
- **Menu URLs:** only http/https/mailto/tel-style schemes, `/path` (not `//` or `/\`) or `#`; control characters and spaces are refused; `esc_url_raw` is applied.
- **Media files:**
  - MIME allow-list intersected with the core-allowed types for the user, checked against the real content (`wp_check_filetype_and_ext`);
  - no SVG by default;
  - `sanitize_file_name` (no traversal, double extensions defused);
  - streamed size cap;
  - Base64 size checked before decoding;
  - temp files removed on every path;
  - download and preview only read files `realpath`-inside uploads;
  - caption and description go through `wp_kses_post`.
- **Error handling:**
  - Throwables are mapped to `internal_error`, details only go to the PHP log;
  - the sideload error is hidden;
  - no SQL or path text reaches the client (apart from the M3 download messages).
- **SQL:** every query is `$wpdb->prepare()`d. The only interpolations are table names and the constant status list; the log filters are whitelisted.
- **Change log:** holds titles and references only, no content. Admin list output is escaped; revision links are gated by `edit_post` on the parent.
- **Admin UI:**
  - every admin-post action has `manage_options` plus `check_admin_referer` (revoke nonce per UUID, clear-log);
  - settings go through `options.php` with `settings_fields` nonces and sanitize callbacks;
  - `closedpostboxes` is core AJAX with its nonce;
  - notices and tabs come from fixed maps;
  - everything is escaped (`esc_js`+`esc_attr` for confirm handlers).
- **Site Health and admin notices:** static text only, escaped; the loopback check is cached and only runs on demand.
- **Guides:** topic slug regex plus the glob'd file list (no traversal); filtered guides are site-owned.
- **Divi pipeline integration:**
  - the kses window covers only `RevisionPoint::capture` plus the one `wp_update_post` and is restored in `finally`, only when kses was active;
  - Divi's own `wp_insert_post_data` security filters stay (verified: they strip `on*` attributes);
  - the insert, update, move, delete and duplicate-node tools only write through Composer/TreeEditor;
  - duplicate-node adds only the stored label plus a constant;
  - replace-text re-checks targets per post;
  - DynamicContentMap drops `loop_*` options and types come from the registry; the fixed rules (enable_html, meta keys, markers) remain.
- **Uninstall and deactivation:** both remove their own options, transient, cron and table on every site of a network, never posts.
- **Limits and DoS:**
  - cursor offset capped at 9 digits, page size ≤100;
  - batch caps (media-update 50, divi-update 50, insert/delete 20, replace-text 20 targets / dry run ≤100 pages, content-create `divi` 30 sections);
  - render summary strings bounded;
  - text search limited to 200 characters.