Skip to content

Enforce 'DeleteSandbox' Permission Set requirement for Sandbox Deletion#1565

Open
laissaouibrahim wants to merge 3 commits intosalesforcecli:mainfrom
laissaouibrahim:blaissao-enforce-ps-requirment-for-sandbox-deletion
Open

Enforce 'DeleteSandbox' Permission Set requirement for Sandbox Deletion#1565
laissaouibrahim wants to merge 3 commits intosalesforcecli:mainfrom
laissaouibrahim:blaissao-enforce-ps-requirment-for-sandbox-deletion

Conversation

@laissaouibrahim
Copy link
Copy Markdown

@laissaouibrahim laissaouibrahim commented Dec 14, 2025

Summary
As requested in IdeaExchange: Add system permission requirement for sf org delete sandbox command, we need to add a security layer to prevent accidental or unauthorized sandbox deletions via the CLI.

Problem
Currently, any user who has authenticated to a sandbox via the CLI can run sf org delete sandbox and delete it. There is no granular permission check within the Salesforce org to restrict this destructive action.

Proposed Solution
Modify the org delete sandbox command to enforce a permission check against the target sandbox before proceeding with deletion.

Acceptance Criteria

  1. Permission Check : The command must verify if the authenticated user has the 'DeleteSandbox' Permission Set assigned in the target org.
  2. Blocking: If the user does not have this Permission Set, the command must fail with an insufficientPermissions error and NOT delete the sandbox.

Implementation Details

  • Implement a hasPermission() method in the command class.
  • create a new permissionset :
  • image
  • Query the PermissionSetAssignment object to check for the assignment of a Permission Set named 'DeleteSandbox' for the current user.

Testing

  • Missing assigned PermissionSet to current user :
image
  • User with assigned PermissionSet :
image
  • Another error :
image

@laissaouibrahim laissaouibrahim requested a review from a team as a code owner December 14, 2025 11:26
@salesforce-cla
Copy link
Copy Markdown

Thanks for the contribution! Before we can merge this, we need @laissaouibrahim to sign the Salesforce Inc. Contributor License Agreement.

@laissaouibrahim
Copy link
Copy Markdown
Author

Thanks for the contribution! Before we can merge this, we need @laissaouibrahim to sign the Salesforce Inc. Contributor License Agreement.

done !

Copy link
Copy Markdown

@ZLeventer ZLeventer left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this — sandbox deletion without authorization checks has bitten a lot of teams. I have some substantive concerns about the implementation before this merges.

The permission check is on the wrong org

org.delete() in @salesforce/core deletes a sandbox by calling the production org's Tooling API — specifically it operates on the SandboxInfo or SandboxProcess sObject via the parent org's connection. The username passed to Org.create() in this PR is the sandbox org itself. So the PSA check runs against the sandbox, but the authorization that actually matters is in the production (parent) org. A user could have DeleteSandbox assigned in the sandbox and still get INSUFFICIENT_ACCESS from production, or be a System Admin in production with no PSA in the sandbox and get incorrectly blocked.

The PR would need to resolve the production org (available as sandboxOrg.getField('isSandbox') → then walk to loginUrl-matched prod org, or via AuthInfoparentUsername) and run the permission query there.

DeleteSandbox is not a standard Salesforce PermissionSet

This PSA doesn't exist in any Salesforce org by default. Every customer would need to manually create a PermissionSet named exactly DeleteSandbox and assign it before this command works — it would fail silently for everyone who doesn't know to do this (the outer catch swallows the connection/query error and continues).

The real Salesforce permission controlling sandbox management is the "Manage Sandboxes" system permission (SystemPermissions.ManageSandboxes). You can check it via:

SELECT PermissionSet.Name, PermissionSet.ManageSandboxes
FROM PermissionSetAssignment
WHERE AssigneeId = '<userId>'
AND PermissionSet.ManageSandboxes = true

Or via UserEntityAccess / UserAppMenuCustomization for a cleaner capability check. Either approach targets a real, documentable Salesforce permission rather than a bespoke convention.

The soft-fail design undermines the security intent

} catch (error) {
  // For other errors (e.g., org connection issues), log a warning but continue
  this.warn(messages.getMessage('warning.couldNotVerifyPermissions', [username]));
}

If the goal is to gate sandbox deletion on authorization, falling through on connection/query failure defeats the feature. A user who can trigger an org connection error (stale token, wrong endpoint, network issue) bypasses the check entirely. If this is a soft advisory — "we'll warn you but won't block you" — then it shouldn't be called a permission requirement. The design should pick one: hard gate that denies on uncertainty, or advisory warning that never throws. Right now it claims to be both.

Error re-throw detection will break on message changes

if (errorMessage.includes('required permission') || errorMessage.includes('DeleteSandbox')) {
  throw error;
}

This checks English message text to decide whether to re-throw. It'll silently swallow the insufficientPermissions error if the message template changes or if Messages ever localizes output. The correct pattern is to set a name on the SfError and check error.name === 'insufficientPermissions' — or just check error instanceof SfError unconditionally and rethrow, only catching non-SfError types as "connection problems."

Org.create is called twice for the same org

sandboxOrg is created at line ~63 for the permission check, then org is created again at line ~87 for org.delete() with the same aliasOrUsername. The first instance should be reused.

No tests

The diff has no changes to test/commands/org/delete/sandbox.ts. The permission check path, the error re-throw path, and the soft-fail warning path are all untested.


The underlying ask from the IdeaExchange is valid and worth implementing. The cleanest path forward IMO: remove the pre-check, and instead catch INSUFFICIENT_ACCESS_OR_READONLY / SandboxAccessDenied from the existing org.delete() call and surface it with an SfError whose .actions points to "Manage Sandboxes" in the production org's profiles/PSAs. That way the authorization check runs against the right org (production), uses a real Salesforce error code rather than a synthetic PSA query, and doesn't require any customer setup.

@laissaouibrahim laissaouibrahim force-pushed the blaissao-enforce-ps-requirment-for-sandbox-deletion branch from 2df6adf to 4d15afb Compare April 25, 2026 11:22
@laissaouibrahim
Copy link
Copy Markdown
Author

Thanks @ZLeventer for the thorough review! I've pushed a commit addressing all the points raised. Here's a summary of what was fixed:

1. Permission check is now on the production org verifyDeletePermission now resolves prodOrgUsername from the sandbox config and queries PermissionSetAssignment against the production org's connection.

2. No more soft-fail — it's a hard gate Removed the try-catch that was swallowing errors with a warning. Any failure in the permission check now propagates and aborts the command.

3. No more string-based error detection Replaced errorMessage.includes(...) with messages.createError() producing properly named SfError instances. Error handling now uses error.name / errorCode.

4. Org.create no longer duplicated Now called once for the production org (permission check) and once for the sandbox org (deletion) — two different orgs, both calls necessary.

5. Tests added
Covered permission granted, permission denied (InsufficientPermissionsError), and missing "Manage Sandboxes" (InsufficientAccessError) scenarios.

6. Successfull unit tests :
image
image
image

Regarding the DeleteSandbox PermissionSet (point 2 from review)
I intentionally chose a custom DeleteSandbox Permission Set rather than relying on the existing "Manage Sandboxes" system permission. Here's why :
In practice, most organizations have their admin users in production with the "Manage Sandboxes" permission already enabled by default on the System Administrator profile. This means relying on it as a security gate provides no real protection — it would effectively allow any admin to delete sandboxes, which is exactly the problem we're trying to solve.

The risk is real : a junior admin can accidentally delete a sandbox through a mistyped command or a careless action, without even realizing the impact. We cannot rely on a permission that is already checked by default for all admin profiles to act as a meaningful safeguard for a destructive operation like sandbox deletion.

Furthermore, the description of "Manage Sandboxes" in the profile configuration reads: "Create and edit sandbox and sandbox templates." — it doesn't even mention deletion. The permission was designed for sandbox creation and configuration, not as an authorization gate for irreversible destructive actions.

This is precisely why the implementation introduces a dedicated, opt-in DeleteSandbox Permission Set that admins must explicitly create and assign. It provides a clear, granular, and intentional authorization mechanism — an admin has to consciously grant deletion rights to specific users, rather than it being inherited from a broad default profile.

The native INSUFFICIENT_ACCESS_OR_READONLY from org.delete() is still caught and surfaced with a clear error message, so both layers of access control are covered: the custom pre-check for organizations that want fine-grained control, and Salesforce's built-in server-side enforcement as a fallback.

Copy link
Copy Markdown
Author

@laissaouibrahim laissaouibrahim left a comment

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

@ZLeventer ZLeventer left a comment

Choose a reason for hiding this comment

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

The hard-gate refactor, named SfErrors, and tests are all solid improvements — thank you for the thorough response.

Conceding the Org.create point — I misread the original; you're right that the two calls target different orgs (prod for the PSA check, sandbox for deletion). That's correct as-is.

The prodOrgUsername silent skip is still a soft fail

if (!prodOrgUsername) {
  return; // ← allows deletion with no permission check
}

If the sandbox config is missing prodOrgUsername — which is common for sandboxes authenticated directly rather than through org create sandbox — the permission check silently passes and deletion proceeds. That's the same failure mode as the original catch-and-continue. Consider throwing here instead, or at minimum logging a visible warning so the user knows the check was skipped.

Still concerned about the custom PSA approach

I understand the argument — ManageSandboxes is broadly assigned to System Admin profiles and isn't granular enough. That's a fair observation. But the consequence of a custom PSA is that sf org delete sandbox works for nobody out of the box. Every team on the planet would hit InsufficientPermissionsError until their admin creates a PermissionSet named exactly "DeleteSandbox" and assigns it. The error message's action item ("Ask your administrator to create a Permission Set named 'DeleteSandbox'...") describes a manual prerequisite that doesn't exist in any Salesforce org today.

The right lever for "we want to restrict who can delete sandboxes" is the production org's profile/PSA setup — revoke ManageSandboxes from profiles that shouldn't have it, and grant it via a custom PSA to those that should. That's the native Salesforce mechanism for exactly this problem. The CLI shouldn't need to invent a parallel convention.

The cleanest path that solves the real problem without the discoverability issue: gate on ManageSandboxes (catch INSUFFICIENT_ACCESS_OR_READONLY from org.delete() + check PermissionSet.ManageSandboxes = true), and document clearly in the error actions that admins can restrict deletion rights by removing ManageSandboxes from the System Administrator profile. That way the command works today for anyone with legitimate delete rights and blocks those without.

Happy to discuss further if there's a use case the ManageSandboxes approach can't cover.

@laissaouibrahim
Copy link
Copy Markdown
Author

Thanks @ZLeventer for the follow-up!

prodOrgUsername silent skip fixed — now throws a MissingProdOrgError instead of silently returning. No deletion proceeds without a verified permission check.

Keeping the custom DeleteSandbox PS — I understand the discoverability concern, but the configuration overhead of each approach is worth comparing:

With ManageSandboxes : You can't remove ManageSandboxes from the standard System Administrator profile — it's baked in. To actually restrict sandbox deletion, an org admin would need to:

  1. Clone the System Administrator profile into a custom profile
  2. Remove ManageSandboxes from that custom profile
  3. Reassign all admin users to the new custom profile
  4. Create a new Permission Set with ManageSandboxes
  5. Assign it only to users who should retain sandbox management rights
  6. That's a disruptive, org-wide change affecting every admin user.

With the custom DeleteSandbox PS :

  1. Create a Permission Set named DeleteSandbox
  2. Assign it to the users who need delete rights

Two steps, no disruption to existing profiles or user assignments.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants