Shared folders, heartbeat widget, and heartbeat-pipeline hardening

[AllTerrainDeveloper]

Shared.folders.mov

Shared folders + Heartbeat widget

Two new features for Desktop Mode.

Shared folders

Owners can share any folder with specific users or whole roles, at
read or read+write capability. Recipients accept, deny, or leave;
their decisions are private per-user (no "first to click decides
for everyone in the role").

Capability model. Owner → write. Direct user invite or accepted
role invite → grant on the shares table. Sub-folders inherit the
parent's grant via the ancestor walker. share_mode='all' still
works for site-wide folders and defaults to read.

Shared namespace. Every recipient sees the same icons inside a
shared folder; only (x, y) is per-user. Files the viewer can't
can_read (private posts, drafts, …) ship with accessGated: true
and render with a lock overlay + permission-explainer preview pane.

REST surface.

Method	Path
GET	/folders/{id}/shares
POST	/folders/{id}/shares
PATCH	/folders/{id}/shares/{shareId}
DELETE	/folders/{id}/shares/{shareId}
POST	/folders/{id}/shares/{shareId}/accept
POST	/folders/{id}/shares/{shareId}/deny
POST	/folders/{id}/leave
GET	/files/users/search

Writes optionally accept If-Match: <updatedAtMs> and return a
structured 409 with reason / actor / current on conflict.

UI.

• Owner: title-bar People button + Share Settings modal
  (<wpd-modal>, <wpd-user-search>, <wpd-role-picker>,
  <wpd-segmented> for read / read+write per recipient, "Apply to
  sub-folders" checkbox).
• Recipient: invitation modal on pending invites, "Leave shared
  folder" tile action, shared folder windows append "· Shared" to
  the title.
• Destructive affordances are gated on a server-computed canTrash
  flag — read-only recipients don't see "Move to Trash" or "Move
  folder to Trash" and the recycle-bin drop target refuses their
  drags.

Schema. v9 adds wp_desktop_mode_folder_shares (polymorphic via
target_type for future non-folder sharing) and
wp_desktop_mode_share_user_decisions (per-user state for
role-principal). Migration is idempotent.

Hooks (Experimental).

• Actions: desktop_mode_files_share_{invited,accepted,denied,revoked,capability_changed,leave}.
• Filters: desktop_mode_files_share_{eligible_roles,can_manage,user_query_args}, desktop_mode_folder_share_{accept_default_parent,user_capability}.
• Activity channels: desktop-mode/folder-share-*.

Heartbeat widget

A Pixi-rendered heart that beats in sync with the WordPress
Heartbeat. Ships as its own lazy Vite bundle
(widget-heartbeat[.min].{js,css}, ~8.7 KB JS) — PIXI loads on
demand through wp.desktop.loadModules(['pixijs']).

• Resting pulse + dramatic big-beat envelope on each real
  heartbeat-tick, with a halo glow and a brief horizontal squish
  during contraction.
• Centred WP logo, real CSS-style gradient (canvas-textured + masked
  through the heart polygon).
• Progress bar counts down to the next tick; survives widget
  re-mounts via a createSharedStore slot.
• Right-click → "Show heart" toggles between the full 310 × 230
  frame and an 88 px compact frame.

OS Setting. New row in OS Settings → Features: WordPress
Heartbeat rate (15 s / 30 s / 45 s / 60 s default). Maps onto
wp.heartbeat.interval('standard' | 'slow') live and persists via
the heartbeat_settings filter.

Tests

• PHPUnit @group desktop-mode-files: 125 / 360 assertions.
• Vitest: layer + drag + shortcuts-sync coverage for the new
  surfaces.
• npm run lint, tsc --noEmit, npm run build clean.

[github-actions]

✅ WordPress Plugin Check Report

✅ Status: Passed

📊 Report

All checks passed! No errors or warnings found.

---

🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

[epeicher]

@claude can you review this PR?

[claude]

Claude finished @epeicher's task in 4m 24s —— View job

---

PR Review: Shared folders, heartbeat widget, and heartbeat-pipeline hardening

• Gather PR context and changed files
• Review PHP backend (sharing, schema, REST)
• Review TypeScript frontend (shares store, UI components, drag)
• Review new web components (wpd-modal, wpd-user-search, wpd-role-picker)
• Review heartbeat widget
• Review tests and docs
• Post comprehensive review

---

This is a substantial, well-structured PR. The capability model, schema design, and security posture are all sound. Here is a detailed review organized by severity.

---

Architecture & Design

The two-table approach (folder_shares + share_user_decisions) is the right design choice. Separating per-principal grants from per-user opt-in state correctly solves the "first role member to click shouldn't decide for everyone" problem. The polymorphic target_type column is a smart forward-compatible hook.

The canTrash server flag approach — computing it at shape time and propagating to the client so it can veto drags and context menu items preemptively — is the right pattern. It mirrors what good REST APIs do (capability hints in the payload), avoids optimistic 403s in the console, and keeps the gate logic single-sourced on the server.

The createSharedStore usage in shares-store.ts:37 is correct per AGENTS.md guidelines, and the heartbeat widget's state persistence across mounts via wpBeatStore is exactly the pattern the docs prescribe.

---

Issues Found

Medium

1. Schema version mismatch in inline comments (includes/desktop-files/schema.php:129)

The constant is DESKTOP_MODE_FILES_SCHEMA_VERSION = '9' and the PR description says "v9 adds…" but the inline comment at line 129 says // Schema v8 (since 0.18.0). This will confuse the next person debugging a migration.

// Schema v8 (since 0.18.0): per-principal grants.

Should be v9. Fix this →

2. If-Match conflict response attributes modification to the wrong user (includes/desktop-files/rest.php:532)

$actor_id = isset( $row['user_id'] ) ? (int) $row['user_id'] : ( isset( $row['owner_id'] ) ? (int) $row['owner_id'] : 0 );

There is no updated_by column tracking who last mutated the row, so the 409 conflict toast blames "whoever currently owns the row" rather than "the session that won the race." In a shared-write scenario (User A owns the folder, User B has write access and moves a file, User C gets the conflict), User C's toast will say "User A moved this" when it was actually User B. Not a correctness bug but a misleading UX. Worth a comment at minimum, or a last_modified_by column addition if conflicts in shared folders are a real workflow concern.

---

Low

3. capability REST args missing enum declaration (includes/desktop-files/rest.php:142, 153)

'capability' => array( 'type' => 'string', 'default' => 'read' ),
// and
'capability' => array( 'type' => 'string', 'required' => true ),

The validation is handled inside desktop_mode_folder_share_invite / desktop_mode_folder_share_update_capability, but declaring the enum in the REST args gives the WP schema free auto-rejection with a cleaner error before the callback fires, and lets wp rest-api scaffold and REST API introspection surfaces describe the field correctly. Fix this →

4. desktop_mode_files_compute_heartbeat_delta docblock missing $shares_version param (includes/desktop-files/heartbeat.php:107)

The function signature at line 107 accepts a 5th $shares_version = 0 parameter, but the @param docblock only lists 4 params. Small doc gap that will surface in IDE hover hints. Fix this →

5. wpd-modal title attribute collision with native HTMLElement.title (src/ui/components/wpd-modal/wpd-modal.ts:178)

const this as unknown as { title: string | null } ).title ?? '';

title is listed in static props which maps to this.getAttribute('title') via the component base. But HTMLElement.title is already a reflected IDL attribute — reading this.title will always return the content of the title attribute (as a string, never null). The cast to { title: string | null } is therefore a red herring (it can't be null). The simpler this.getAttribute('title') ?? '' documents the intent clearly without the confusing cast.

6. Ancestor-walk query count under cold cache (includes/desktop-files/shares-store.php:965)

desktop_mode_folder_share_user_capability_cascade calls desktop_mode_folder_share_user_capability_direct per ancestor, which itself fires 2 queries (user-shares, role-shares). At the 16-ancestor cap that's up to 32 queries per capability check. On sites with a WP object cache this is fine; on plain-file/no-cache installs with deeply-nested shared folders, this chain fires on every heartbeat delta and every shape_placement call. A future optimization (batch ancestor IDs into a single IN query) would help, but this is not a correctness issue so it can land as-is with a @todo comment if you expect deep nesting.

7. ingestPendingInvites sets mutated = true even on no-op updates (src/desktop-files/shares-store.ts:89)

// Replace existing snapshot (server may have updated cap).
s.state.pending = s.state.pending.map( ( p ) => ( p.id === inv.id ? inv : p ) );

mutated = true is set even if the replacement is byte-for-byte identical (server hasn't changed anything). This triggers an unnecessary s.notify() and a re-render pass. The fix is to compare the incoming invite against the existing snapshot before flipping the flag, but given the low frequency of heartbeat ticks, this is a minor efficiency note rather than a real issue.

---

Security Observations (all clear)

• All capability checks (can_manage, principal_matches_user, share_gate_trash) are server-enforced. No client-only gating.
• All DB writes use parameterized $wpdb->prepare() — including the dynamic IN (…) clause at line 1173-1183, which is safe because the placeholders array matches count($roles).
• /files/users/search is gated behind edit_posts at the permission callback level, not just inside the handler.
• The innerHTML at share-settings-modal.ts:467 uses only the controlled ternary capLabel = invite.capability === 'write' ? 'Read + Write' : 'Read' — no user-supplied strings interpolated.
• modal.setAttribute('title', …) calls with server-supplied strings are safe (attribute API, not innerHTML).
• The $shares_sql UNIQUE KEY on (target_type, folder_id, principal_type, principal_ref) is a solid race-condition backstop for concurrent invite attempts.

---

Positive Observations

• The heartbeat widget correctly uses window.wp.desktop.loadModules(['pixijs']) rather than importing Pixi directly, keeping the widget bundle footprint minimal and respecting the module registry boundary.
• The bootWpBeatTracker singleton + createSharedStore-backed state means the countdown progress survives widget re-mounts without a visible restart — good UX attention to detail.
• The desktop_mode_files_migrate_share_meta_to_shares() idempotent backfill pattern (guarded by a separate migration-option key) is correctly done and the PHPUnit test at test_migration_backfills_share_meta_users covers it.
• The DropTarget.accept() hook that inspects placement.canTrash !== false and uses !== false (not === true) as the default-allow semantic is the right choice for backwards compatibility.
• MutationObserver on #desktop-mode-area as a belt-and-braces bin-tile re-discovery is smart defensive coding.
• Test depth is excellent: 824 lines of PHPUnit (schema, invite/accept/deny/revoke lifecycle, capability gating, REST routes, 409 conflict, heartbeat pending, scoped trash) plus Vitest for layer, drag-manager, and shortcuts-sync.

---

Overall: ready to merge with minor fixes. Items 1 (schema version comment) and 3 (enum declaration) are the cleanest to address before merging. The rest can be filed as follow-up issues.

[AllTerrainDeveloper]

@claude could you do another review? Can you focus on security, database and plugin checks?

[claude]

Claude finished @AllTerrainDeveloper's task in 4m 51s —— View job

---

Security, database, and plugin check review — focused pass on behalf of @AllTerrainDeveloper.

• Review PHP REST endpoints — capability gates, nonce checks, permission callbacks
• Review SQL queries — injection risks, parameterization, dynamic clauses
• Review database schema — migration safety, index correctness, collision risks
• Review data sanitization and output escaping
• Review plugin check compliance (WordPress standards)

---

Corrections to the previous review

Before the new findings: two items from the prior review were already correct in the code.

Issue #1 (schema comment "v8") — The schema file at line 129 currently reads // Schema v9 — per-principal grants… and line 181 reads // v10: updated_by column…. The version constant is '10'. The comments accurately describe which schema version introduced which feature; nothing needs changing.

Issue #3 (missing enum on capability args) — Looking at the actual REST registration code, enum => array( 'read', 'write' ) is already declared on both the POST args (line 148) and the PATCH args (line 163). This was not missing.

---

Security

Medium — Revoked share still visible via legacy share_meta fallback (includes/desktop-files/sharing.php:175–185)

This is the most impactful security issue in the PR.

When desktop_mode_folder_share_revoke runs, it deletes the row from folder_shares and from share_user_decisions, then bumps updated_at_ms on the folder. It does not modify the folder row's share_mode or share_meta columns.

desktop_mode_files_user_can_see_folder checks the new share table first (desktop_mode_folder_share_user_capability), but if that returns 'none' — which it will after revocation — it falls back to evaluating the legacy share_meta JSON:

// sharing.php:175-185
$cap = desktop_mode_folder_share_user_capability( (int) $folder['id'], (int) $user_id );
$can = 'none' !== $cap;
if ( ! $can ) {
    $meta = is_array( $folder['share_meta'] ) ? $folder['share_meta'] : array();
    if ( 'users' === $mode && isset( $meta['users'] ) ) {
        $users = array_map( 'intval', $meta['users'] );
        $can   = in_array( (int) $user_id, $users, true );
    }
    // … role path likewise
}

Attack path: A folder shared pre-v9 (or migrated by desktop_mode_files_migrate_share_meta_to_shares) retains share_mode='users' and share_meta.users=[uid]. After migration the owner revokes that user via the new modal. The row is removed from folder_shares; desktop_mode_folder_share_user_capability returns 'none'; but the legacy fallback reads share_meta.users, finds the id, and re-grants visibility. The revoked user continues to see the folder — they see live placement updates via the heartbeat, and can open the folder.

The problem is that a hard delete from folder_shares is indistinguishable from "never been invited" at the legacy-fallback layer. A 'revoked' terminal state (rather than deleting the row) would give the capability resolver a signal to short-circuit before reaching the fallback. Alternatively, desktop_mode_folder_share_revoke should scrub the user/role from share_meta on the folder row, or the migration should zero out share_meta on folders it processes. Fix this →

Low — 409 conflict response exposes actor display name + avatar to read-only recipients (includes/desktop-files/rest.php:574–593)

The desktop_mode_files_check_if_match function builds a 409 payload that includes actor.name and actor.avatar. This is useful for the shared-write UX ("User X moved this"), but a read-only recipient who hits a stale If-Match on a placement inside a shared folder will receive the display name and avatar of whoever last modified the row — even if that person's identity is otherwise not known to them. In multi-tenant or high-privacy deployments this is a mild PII leak. Worth documenting explicitly in the hooks reference as intentional behaviour, or gating the actor fields behind an edit_posts check.

Low — Double-$wpdb->prepare() pattern in heartbeat placement query (includes/desktop-files/heartbeat.php:151–169)

$where_parts = array( $wpdb->prepare( 'user_id = %d', $user_id ) );
if ( ! empty( $visible_folder_ids ) ) {
    $placeholders = implode( ',', array_fill( 0, count( $visible_folder_ids ), '%d' ) );
    $where_parts[] = $wpdb->prepare( "parent_id IN ($placeholders)", $visible_folder_ids );
}
$where_sql = '(' . implode( ' OR ', $where_parts ) . ')';
$rows = $wpdb->get_results(
    $wpdb->prepare(
        "SELECT * FROM {$tables['placements']} WHERE $where_sql AND ...",
        $placements_version, $cap
    ),

Each component is prepared individually, then the resulting strings are concatenated and passed to a second $wpdb->prepare(). This works correctly today because neither inner-prepared string can contain a literal % character (both sides produce plain integers). However WP 5.3+ will issue a _doing_it_wrong() notice if it detects % characters in the SQL passed to prepare() that don't map to a placeholder. The pattern is fragile. The safer approach is to build the full SQL string once and call prepare() once. Fix this →

---

Database

Medium — desktop_mode_files_purge_folder leaves orphan rows in folder_shares and share_user_decisions (includes/desktop-files/trash.php:999–1004)

$wpdb->delete( $tables['placements'], array( 'trashed_via_folder' => $folder_id ), array( '%d' ) );
$wpdb->delete( $tables['folders'],    array( 'id'                => $folder_id ), array( '%d' ) );

There are no corresponding deletes from $tables['shares'] or $tables['decisions']. The orphan rows are harmless to the access-control model (the folder row is gone so no auth check will reach these rows) but:

1. They accumulate indefinitely on sites that frequently create and purge shared folders.
2. The heartbeat's desktop_mode_files_get_pending_shares_for_user JOINs against folders f ON f.id = s.folder_id AND f.trashed_at_ms IS NULL — this naturally suppresses invites for purged folders, but the suppression relies on the folder row being absent, not on an explicit cleaned state.
3. If BIGINT UNSIGNED AUTO_INCREMENT wraps around on folder_shares.id in an extreme long-running install, a reused folder-id could match stale share rows. Theoretical only, but easy to prevent now.

The fix is two $wpdb->delete() calls at the end of desktop_mode_files_purge_folder, mirroring the desktop_mode_folder_share_revoke pattern. Fix this →

Low — desktop_mode_files_dedupe_placements has no per-operation idempotency guard (includes/desktop-files/schema.php:254–273)

The function runs every time desktop_mode_files_install_schema() is called. The outer guard is the version-option comparison, but if the migration aborts after bumping the version (e.g. a PHP timeout on the ALTER TABLE calls), the deduplication DELETE-JOIN will re-run against a potentially large table on the next page load. Every other destructive one-shot step in this file already uses its own option key (DESKTOP_MODE_FILES_SHARES_MIGRATED_OPTION). Wrapping dedupe_placements in the same pattern makes it safe to re-run after a partial failure. Fix this →

Low — ALTER TABLE ADD COLUMN called from high-frequency hooks without advisory locking

desktop_mode_files_maybe_install_schema is registered on admin_init, rest_api_init, and init. The INFORMATION_SCHEMA check that guards each ALTER TABLE in the ensure_* helpers is not atomic with the ALTER itself (classic TOCTOU). On a site with several PHP workers hitting admin or REST simultaneously after a plugin update, two workers could both pass the column-exists check before either runs the ALTER. On MySQL/MariaDB the second ALTER on an already-present column raises an error; $wpdb->query() swallows it silently (no return-value check). The runtime result is correct but the error log will have noise. Adding OR IF NOT EXISTS syntax to the ALTER (MySQL 8.0+/MariaDB 10.0+) eliminates this cleanly. For older MySQL 5.7 the existing INFORMATION_SCHEMA + suppress-errors approach is the standard WP pattern — worth documenting inline.

---

Plugin check / WordPress standards

Correctly passing ✅

• All $wpdb->query/insert/update/delete calls use proper $wpdb->prepare() with typed placeholders.
• Dynamic IN (…) clauses (pending shares for user, purge stale tombstones, visible folder placements) build their placeholder arrays with array_fill(0, count($vals), '%s/%d') and pass $vals to prepare(). The phpcs:ignore comments on these are correctly scoped and justified.
• Capability checks on every REST route: desktop_mode_files_rest_permission() guards all file routes; desktop_mode_files_rest_search_users_permission() adds edit_posts for the user-search endpoint. No route is open without authentication.
• No window.confirm/window.alert/window.prompt — all UI uses wpdConfirm.
• All REST args exposed to user input are sanitized server-side (sanitize_text_field on folder name, (int) casts on IDs, enum validation on principal_type, capability, state).
• heartbeatRate in OS settings is validated against array( 15, 30, 45, 60 ) — no arbitrary interval can be injected by a user.
• share_meta JSON written to the DB always goes through wp_json_encode() and never via interpolation.
• No innerHTML used with server-supplied strings in share-settings-modal.ts; the only interpolated HTML uses the controlled ternary capLabel = invite.capability === 'write' ? 'Read + Write' : 'Read'.
• Plugin Check CI: passes (confirmed in prior comment).

Low — Heartbeat widget CSS enqueued for all desktop-mode users (includes/widgets/heartbeat.php:99–105)

admin_enqueue_scripts adds the 1.9 KB stylesheet unconditionally for any user with desktop mode enabled, whether or not they've added the widget. The JS bundle stays lazy (correct). Given the file size this is negligible, but it diverges from the lazy-load pattern the rest of the widget system follows. An alternative is to inject the stylesheet inline from the widget JS when the widget frame mounts — same approach Pixi uses via loadModules. Worth a follow-up issue, not a blocker.

---

Summary

Severity	Area	Issue
Medium	Security	Revoked share re-granted via legacy share_meta fallback (sharing.php:175)
Medium	Database	purge_folder leaves orphan rows in folder_shares / share_user_decisions (trash.php:999)
Low	Security	409 actor PII visible to read-only recipients (rest.php:578)
Low	Security	Double-prepare() in heartbeat query (heartbeat.php:151)
Low	Database	dedupe_placements lacks per-operation idempotency guard (schema.php:254)
Low	Database	ALTER TABLE TOCTOU on concurrent workers (all ensure_* helpers)
Low	Plugin	Heartbeat widget CSS eagerly enqueued for all users (heartbeat.php:99)

The two Medium items should be resolved before merge. The share visibility bypass is the most important — it silently undermines the revocation UX the entire sharing feature is built around.

[AllTerrainDeveloper]

@claude could you do another review? Can you focus on security, database and plugin checks?

[claude]

Claude finished @AllTerrainDeveloper's task in 6m 39s —— View job

---

Security, database & plugin-check review (third pass)

• Gather context and plan
• Review shares-store.php — capability logic, SQL, concurrency
• Review sharing.php — access control, legacy fallback
• Review rest.php — permission callbacks, sanitization, nonces
• Review schema.php — migration safety, idempotency
• Review trash.php — cascade deletes, orphan rows
• Review heartbeat.php — SQL parameterization, data exposure
• Review TypeScript — XSS, innerHTML, auth enforcement on client
• Review plugin standards — escaping, enqueue hygiene, capability checks
• Post findings

---

Corrections to the previous two reviews

All six medium-and-low items raised in the prior reviews are addressed in the current code. Before the new findings:

Prior Medium — Revoked share re-granted via share_meta legacy fallback (sharing.php:175):
sharing.php lines 155–165 now read:

// Non-owner viewer: the shares table is the single source
// of truth. `share_meta` on the folders row is diagnostic
// only — it is never consulted for visibility. (Earlier
// drafts had a fallback that silently re-granted access
// to revoked recipients; reviewer caught the
// revocation-bypass and we dropped the fallback before
// the feature shipped.)
$cap = desktop_mode_folder_share_user_capability( (int) $folder['id'], (int) $user_id );
$can = 'none' !== $cap;

✅ Fixed before the feature shipped. The code comment explicitly acknowledges the reviewer's note.

Prior Medium — purge_folder leaves orphan rows in folder_shares / share_user_decisions (trash.php:999):
trash.php lines 999–1029 now cascade-delete decisions and shares rows before deleting the folder:

// Cascade-revoke every share + per-user decision for the folder
// BEFORE deleting the folder row.
$share_ids = (array) $wpdb->get_col( ... );
if ( ! empty( $share_ids ) ) {
    $wpdb->query( $wpdb->prepare( "DELETE FROM {$tables['decisions']} WHERE share_id IN ($placeholders)", ... ) );
    $wpdb->query( $wpdb->prepare( "DELETE FROM {$tables['shares']}   WHERE id IN ($placeholders)",      ... ) );
}

✅ Fixed.

Prior Low — 409 conflict toast exposes actor PII to read-only recipients (rest.php:578):
rest.php lines 636–658 now implement a $viewer_can_see PII gate: actor name + avatar are only included when the requesting viewer owns the row, owns the parent folder, or holds at least read capability on the folder via the shares table. All other viewers receive { id: 0, name: '', avatar: '' }.
✅ Fixed.

Prior Low — Double-prepare() in heartbeat placement query (heartbeat.php:151):
heartbeat.php lines 178–197 now build the full SQL template and all args as a single array_merge(...) then call $wpdb->prepare() once. The previous nested-prepare risk is gone.
✅ Fixed.

Prior Low — dedupe_placements re-runs on every schema call (schema.php:254):
desktop_mode_files_dedupe_placements() now checks INFORMATION_SCHEMA.STATISTICS for the placement_unique index before running the expensive DELETE JOIN: if the index exists, the function returns immediately. This guards the cost more tightly than an option key (and without a second option to manage).
✅ Addressed.

Prior Low — Schema version comment saying "v8" (schema.php:129):
The comment at line 128 now reads // Schema v9 — per-principal grants...; the constant is DESKTOP_MODE_FILES_SCHEMA_VERSION = '10' (v10 added the updated_by column in the same PR). Version labels and comments are consistent.
✅ Fixed.

---

New Findings — Security

Low — user_login (WordPress username) exposed in /files/users/search response

rest.php:965-970 returns:

$out[] = array(
    'id'        => (int) $user->ID,
    'name'      => (string) $user->display_name,
    'login'     => (string) $user->user_login,  // ← WordPress username
    'avatarUrl' => get_avatar_url( $user->ID, array( 'size' => 48 ) ),
);

wpd-user-search.ts:265 renders u.login as a second label in the autocomplete dropdown. The field is gated by edit_posts, but on sites where admins have suppressed author pages or otherwise tried to keep usernames non-public, any author/editor can now enumerate them via this endpoint. The username is never actually needed for the share invite — only the id is submitted to the server. Dropping login from the response, or making it optional via a filter, would avoid the concern without breaking the UI. Fix this →

Low — DROP TABLE in purge endpoint: filter-provided table names are not sanitized

rest.php:869:

$wpdb->query( "DROP TABLE IF EXISTS `{$tbl}`" );

$tbl comes from the desktop_mode_files_sharing_tables_for_purge filter without any sanitization. A backtick embedded in $tbl breaks the identifier quoting (`a`; DROP TABLE wp_users -- becomes valid multi-statement SQL). Exploitation requires both manage_options (to call the endpoint) and PHP code execution (to add the filter), so this is effectively post-compromise only — but a one-line fix eliminates it entirely. Fix this →

---

New Findings — Database / API Design

Low — URL {folder_id} path parameter not cross-validated in share PATCH / DELETE / accept / deny routes

rest.php:764-807 — the four share-mutation handlers (rest_update_share, rest_delete_share, rest_accept_share, rest_deny_share) all route authorization through the share row's actual folder_id, not the {id} from the URL. This is correct for security (you can only modify shares you have access to), but it means the URL's folder ID is silently ignored: PATCH /folders/99/shares/42 succeeds when share 42 belongs to folder 1, provided you own/are-recipient-of that share.

The risk today is zero — the auth checks are sound. The risk tomorrow is confusion for middleware that interprets the URL folder ID for rate-limiting, audit logging, or scoped permissions. A two-line cross-check in each handler would close it:

$row = desktop_mode_files_get_share( $share_id );
if ( $row && (int) $row['folder_id'] !== (int) $req['id'] ) {
    return new WP_Error( 'desktop_mode_files_not_found', ..., [ 'status' => 404 ] );
}

Fix this →

---

Confirmed Clean — Security

• Capability checks: All REST routes have permission_callback. Share management requires can_manage (owner-only by default, filterable). Accept/deny require principal_matches_user. Leave requires the user to not be the folder owner. No route is open without authentication.
• SQL parameterization: All $wpdb->prepare() calls use typed placeholders. Dynamic IN (…) clauses in get_pending_shares_for_user, capability_cascade, purge_stale_tombstones, and purge_folder build placeholder arrays via array_fill(0, count($ids), '%d|%s') and pass the values to a single prepare(). The one NOT IN PREPARED carve-out in capability_cascade uses array_map('intval', ...) so only integers reach the SQL — confirmed safe.
• XSS: The custom html tagged-template renderer (src/ui/core/html.ts) inserts dynamic values via document.createTextNode() (child positions) and setAttribute() (attribute positions). No innerHTML path exists for values. Confirmed safe for u.name, u.login, folder names, and owner names throughout the share UI.
• innerHTML in share-settings-modal.ts:467: The only innerHTML in the sharing UI uses capLabel = invite.capability === 'write' ? 'Read + Write' : 'Read' — fully hardcoded output, no user input interpolated. Safe.
• Heartbeat widget: Pure Pixi canvas rendering. No innerHTML, eval, or document.write. Clean.
• Ancestor-walk cascade: shares-store.php:1028-1035 interpolates $ids_csv directly into SQL without prepare() — this is safe because every element went through array_map('intval', ...) before joining, and the phpcs:ignore is correctly scoped and justified.
• Deletion cascade: desktop_mode_folder_share_revoke cleans decisions rows before leaving; desktop_mode_files_purge_folder now cleans both shares and decisions before deleting the folder row.

Confirmed Clean — Plugin Standards

• All REST args exposed to user input use sanitize_text_field / (int) casts / wp_json_encode at the write boundary.
• share_mode is validated with in_array($mode, desktop_mode_files_share_modes(), true) in both create_folder and update_folder.
• heartbeatRate OS setting is validated against array( 15, 30, 45, 60 ).
• desktop_mode_files_rest_admin_permission() gates the destructive purge route on manage_options — not just edit_posts.
• Plugin Check CI: ✅ passes (confirmed in PR comment from 2026-05-14).

---

Summary

Severity	Area	Item	Status
Low	Security	user_login exposed in user-search response	New
Low	Security	DROP TABLE filter values not sanitized in purge endpoint	New
Low	API design	URL {folder_id} ignored in share PATCH/DELETE/accept/deny	New
Medium	Security	Revoked share re-granted via share_meta fallback	✅ Fixed
Medium	Database	purge_folder orphan rows in shares + decisions	✅ Fixed
Low	Security	409 actor PII leak to read-only recipients	✅ Fixed
Low	Security	Double-prepare() in heartbeat query	✅ Fixed
Low	Database	dedupe_placements idempotency guard	✅ Fixed
Low	Schema	Version comment mismatch	✅ Fixed
Low	Plugin	Heartbeat widget CSS eagerly enqueued for all users	Remains (cosmetic)

The two previously-identified Medium issues are both fixed. The new findings are all Low. The code is well-secured and ready to merge.

[epeicher]

Thanks for adding this great functionality @AllTerrainDeveloper, I love it! I think it can be very useful to share lists of items to review for example.

I have tested it by using different profiles on different browsers and it works perfectly, thanks for addressing all the comments.

LGTM!