Skip to content

Security: reject unsafe manifest plugin paths during site import#1379

Open
vuckro wants to merge 2 commits into
Ultimate-Multisite:mainfrom
vuckro:security/import-manifest-path-hardening
Open

Security: reject unsafe manifest plugin paths during site import#1379
vuckro wants to merge 2 commits into
Ultimate-Multisite:mainfrom
vuckro:security/import-manifest-path-hardening

Conversation

@vuckro

@vuckro vuckro commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

move_and_activate_plugins() builds rename() / activate_plugin() targets
from the plugin keys of the imported site manifest (untrusted JSON inside the
import package). Those keys were used without validation, so a crafted manifest
could use a path-traversal key (e.g. ../../..) to relocate files or activate a
PHP file outside WP_PLUGIN_DIR — a second-order path-traversal /
arbitrary-inclusion risk when a network admin imports a site package from an
untrusted source (e.g. a template marketplace).

Changes

Validate each plugin key before use: reject non-strings, absolute paths, NUL
bytes and any parent-directory (..) segment, skipping them with a warning.

The import action itself remains network-admin-gated; this hardens what a
malicious package can do once an admin chooses to import it.

move_and_activate_plugins() builds rename()/activate_plugin() targets from the
plugin keys of the imported site manifest (untrusted JSON inside the import
ZIP). Those keys were used without validation, so a crafted manifest could use
a path-traversal key (e.g. "../../..") to relocate files or activate a PHP file
outside WP_PLUGIN_DIR — a second-order path traversal / arbitrary-inclusion risk
when importing a site package from an untrusted source (e.g. a template
marketplace).

Validate each plugin key before use: reject non-strings, absolute paths,
NUL bytes and any parent-directory ("..") segment, and skip with a warning.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@superdav42, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 22 minutes and 48 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 29ccc33c-4931-4ca9-bd9f-8d00cb3f7ebc

📥 Commits

Reviewing files that changed from the base of the PR and between bc4d4bb and 33a9556.

📒 Files selected for processing (1)
  • inc/site-exporter/mu-migration/includes/commands/class-mu-migration-import.php
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@superdav42

Copy link
Copy Markdown
Collaborator

Stuck-merge detector: PR has been merge-eligible but unmerged past the threshold

The pulse merge pass has classified PR #1379 as STUCK_OTHER and it has been sitting unmerged longer than AIDEVOPS_MERGE_STUCK_AGE_MINUTES (currently 240m). The deterministic merge gates are evaluated every cycle (~120s) and this PR has consistently failed them.

Failing checks on PR #1379

  • (no FAILURE entries in rollup; check rollup manually)

Worker guidance for the next attempt

  1. Read PR Security: reject unsafe manifest plugin paths during site import #1379 body + the latest check run logs:
    gh pr checks 1379 --repo Ultimate-Multisite/ultimate-multisite
  2. If the failing checks are environment/Setup-step (Format, Lint, Typecheck all FAIL at the same step), the canonical default branch likely has a broken lockfile or a CI infra change — fix at the base, not on this PR. Look for a sibling outage meta-issue in this repo (filed by the same detector) before forking off here.
  3. If the failures are PR-specific (e.g. a Typecheck error introduced by this PR's code), rebase onto the latest default branch and address the diagnosed errors. Use full-loop-helper.sh start from the linked PR's worktree.
  4. If the linked issue body lacks the worker-ready file paths and verification commands required by t1900, post a comment naming the missing context before dispatching another worker — the next attempt will burn tokens on exploration otherwise.

Why you're seeing this

Every pulse cycle (~120s) the deterministic merge pass re-evaluates open PRs. PRs that pass APPROVED + MERGEABLE but fail required checks have historically been re-evaluated silently every cycle until a human noticed. The stuck-merge detector (t3193) surfaces them after AIDEVOPS_MERGE_STUCK_AGE_MINUTES minutes idle. This comment is posted exactly once per linked issue — repeated stuck cycles will NOT spam the thread. If the PR merges and the issue is reopened later with a fresh stuck PR, the marker will allow a second comment.

Posted automatically by pulse-merge-stuck.sh (t3193 / GH#21895). Threshold env: AIDEVOPS_MERGE_STUCK_AGE_MINUTES=240.


aidevops.sh v3.20.57 automated scan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants