Skip to content

fix(security): MagicAI model-fetch SSRF + role permission escalation - #689

Merged
navneetkumar-pim-webkul merged 4 commits into
3.xfrom
fix/magicai-ssrf-role-escalation
Sep 8, 2026
Merged

navneetkumar-pim-webkul merged 4 commits into
3.xfrom
fix/magicai-ssrf-role-escalation

Conversation

@navneetkumar-pim-webkul

Copy link
Copy Markdown
Collaborator

Summary

Two high-severity findings from a full-project security audit, both fixed with minimal, backward-compatible patches.

1. Privilege escalation via role editing (broken access control)

RoleController::store/update wrote the submitted permissions[] verbatim; RoleForm::authorize() blocked only promotion to all. A delegated role manager (custom role holding settings.roles.edit) could edit its own role to grant permissions it did not hold → vertical privilege escalation.

Fix: RoleForm::authorize() now also rejects any permission the acting admin does not itself hold — mirroring the guard already in UserController::roleAssignmentError(). Adds the cannot-grant-unheld-permissions message across all 33 locales.

2. SSRF in MagicAI model discovery

fetchModels validated the submitted api_url with SafeWebhookUrl, then fetched it with a bare Guzzle client that follows redirects and re-resolves DNS. A validated public URL could pivot to an internal host (cloud metadata / RFC1918) via a 30x redirect or DNS rebinding.

Fix: the user-supplied fetches (Custom / Azure / Ollama) now merge SafeWebhookUrl::httpOptions($url) — IP-pinning + no redirect following — matching the already-hardened runtime AiApiClient. Fixed-host provider branches are unchanged.

Backward compatibility

  • Full-access admins and admins granting only permissions they hold are unaffected; only the escalation path now returns 403.
  • Legitimate remote AI providers don't redirect to internal IPs, so model discovery keeps working; only redirect-to-internal / rebind is blocked.

Tests

  • Settings/RolePermissionEscalationTest — self-escalation, other-role escalation, promote-to-all all forbidden; held-permission edit allowed.
  • MagicAI/FetchModelsSsrfTest — validated URL still follows a redirect on a bare client (repro); SafeWebhookUrl::httpOptions blocks it (fix primitive).

Verification

pint, phpstan (level 2), pest, and unopim:translations:check (418/418) all passed on identical code in the working checkout. CI runs the full suite on this clean branch.

…alation

Role create/update wrote the submitted permissions array verbatim and only
blocked promotion to all-access, so a delegated role manager (holding
settings.roles.edit) could edit its own role to grant permissions it did not
hold — vertical privilege escalation. RoleForm::authorize now also rejects any
permission the acting admin does not itself hold, mirroring the assignment
guard already in UserController. Adds the cannot-grant-unheld-permissions
message across all 33 locales.

MagicAI model discovery validated the submitted api_url with SafeWebhookUrl but
then fetched it with a bare Guzzle client that follows redirects and re-resolves
DNS, so a validated public URL could pivot to an internal host (cloud metadata /
RFC1918) via a 30x redirect or DNS rebinding. The user-supplied fetches
(Custom / Azure / Ollama) now merge SafeWebhookUrl::httpOptions — IP-pinning
plus no redirect following — matching the already-hardened runtime AiApiClient.
Fixed-host provider branches are unchanged.

Claude-Session: https://claude.ai/code/session_01MWhxQzQ9X8rqQ8LqXzGeij
Copilot AI lite review requested due to automatic review settings September 8, 2026 11:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

AiProvider references SafeWebhookUrl without importing/qualifying it, which will cause a runtime fatal error in the hardened request path.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR applies two targeted security hardenings in Unopim’s admin area: preventing delegated role managers from self-escalating via role edits, and preventing SSRF pivots (redirect/DNS rebinding) during MagicAI “fetch models” discovery.

Changes:

  • Tighten RoleForm::authorize() to forbid granting permissions the acting admin does not hold (and deny promotion to full-access).
  • Harden MagicAI model discovery requests by applying SafeWebhookUrl::httpOptions() (no redirects + IP pinning when available).
  • Add focused regression tests and propagate a new localized authorization message across locales.
File summaries
File Description
packages/Webkul/MagicAI/src/Enums/AiProvider.php Apply SSRF-guarded HTTP options for user-supplied model discovery URLs.
packages/Webkul/Admin/src/Http/Requests/RoleForm.php Add authorization guard preventing permission self-escalation on role edits.
packages/Webkul/Admin/tests/Feature/Settings/RolePermissionEscalationTest.php New regression coverage for role permission escalation scenarios.
packages/Webkul/Admin/tests/Feature/MagicAI/FetchModelsSsrfTest.php New regression coverage for redirect-based SSRF in model discovery.
packages/Webkul/Admin/src/Resources/lang/ar_AE/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/ca_ES/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/da_DK/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/de_DE/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/en_AU/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/en_GB/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/en_NZ/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/en_US/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/es_ES/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/es_VE/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/fi_FI/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/fr_FR/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/hi_IN/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/hr_HR/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/id_ID/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/it_IT/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/ja_JP/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/ko_KR/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/mn_MN/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/nl_NL/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/no_NO/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/pl_PL/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/pt_BR/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/pt_PT/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/ro_RO/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/ru_RU/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/sv_SE/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/tl_PH/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/tr_TR/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/uk_UA/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/vi_VN/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/zh_CN/app.php Add cannot-grant-unheld-permissions translation.
packages/Webkul/Admin/src/Resources/lang/zh_TW/app.php Add cannot-grant-unheld-permissions translation.
Review details
  • Files reviewed: 37/37 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/Webkul/MagicAI/src/Enums/AiProvider.php
Comment thread packages/Webkul/MagicAI/src/Enums/AiProvider.php Outdated
…st fixture

- Restore the use Webkul\Webhook\Validators\SafeWebhookUrl import in
  AiProvider (dropped when the formatter ran before the usage existed);
  the unqualified reference resolved to the enum's own namespace and broke
  PHPStan.
- Run Pint over the 33 Admin locale files so the added key aligns.
- RoleUserAuthorizationTest: the acting admin now also holds `dashboard`,
  the permission it grants, so the ACL-gating assertion no longer trips the
  new "cannot grant unheld permissions" guard.

Claude-Session: https://claude.ai/code/session_01MWhxQzQ9X8rqQ8LqXzGeij
httpOptions() pins the host to the validated address only when the URL
validates as public; otherwise it returns the redirect guard alone. The
docblock promised pinning unconditionally, which overstates the guarantee
exactly where a reader is reasoning about SSRF.

Claude-Session: https://claude.ai/code/session_01Es54dGoiX1pP64XqxvJmXw
@navneetkumar-pim-webkul
navneetkumar-pim-webkul merged commit f4a9256 into 3.x Sep 8, 2026
21 checks passed
@navneetkumar-pim-webkul
navneetkumar-pim-webkul deleted the fix/magicai-ssrf-role-escalation branch September 8, 2026 13:21
navneetkumar-pim-webkul added a commit that referenced this pull request Sep 9, 2026
Resolves overlap with the MagicAI SSRF (#689) and purifier cache race (#675)
fixes that landed on 3.x independently.

Claude-Session: https://claude.ai/code/session_01JfF9ke2aCFppkN9pYbCfKn
navneetkumar-pim-webkul added a commit that referenced this pull request Sep 17, 2026
Eight merged pull requests since v3.1.0 carried no changelog entry: the
role-escalation and MagicAI SSRF fixes (#689, #690, #691), the export
column selection and category keyword matching fixes (#697, #698), the
media replacement fix (#709), the Gemini endpoint (#699), the channel
deletion fix (#688), the purifier cache race (#675), and the admin UI
and configuration work squashed into #700.

3.1.1 is a patch release, so everything is filed under bug fixes and
improvements; no feature section.

The heading is stamped 3.1.1 to match Core::VERSION, which the branch
already carries.
navneetkumar-pim-webkul added a commit that referenced this pull request Sep 17, 2026
Eight merged pull requests since v3.1.0 carried no changelog entry: the
role-escalation and MagicAI SSRF fixes (#689, #690, #691), the export
column selection and category keyword matching fixes (#697, #698), the
media replacement fix (#709), the Gemini endpoint (#699), the channel
deletion fix (#688), the purifier cache race (#675), and the admin UI
and configuration work squashed into #700.

3.1.1 is a patch release, so everything is filed under bug fixes and
improvements; no feature section.

The heading is stamped 3.1.1 to match Core::VERSION, which the branch
already carries.
sandeepp-webkul pushed a commit to sandeepp-webkul/unopim that referenced this pull request Sep 21, 2026
* fix: validate empty files during product import instead of throwing code error (unopim#696)

* fix: hide broken logo image in profile dropdown when remote fetch fails (unopim#700)

* fix: disable @ attribute suggestions in profile image AI generation (unopim#701)

* fix: set HTMLPurifier cache path to storage directory to prevent vendor write error during import (unopim#350)

* fix: hide webhook logs tab and enforce ACL for unauthorized roles (unopim#545)

* fix: EditImage tool now fetches product image by SKU instead of requiring upload (unopim#683)

* fix: add XLSX export support to AI Agent ExportProducts tool (unopim#684)

* fix: add SKU validation to AI Agent bulk import to reject special characters (unopim#689)

* style: apply pint formatting to AiAgent lang file

* fix: address copilot review on PR unopim#353
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