Enforce 'DeleteSandbox' Permission Set requirement for Sandbox Deletion#1565
Conversation
|
Thanks for the contribution! Before we can merge this, we need @laissaouibrahim to sign the Salesforce Inc. Contributor License Agreement. |
done ! |
ZLeventer
left a comment
There was a problem hiding this comment.
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 AuthInfo → parentUsername) 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 = trueOr 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.
2df6adf to
4d15afb
Compare
|
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 Regarding the DeleteSandbox PermissionSet (point 2 from review) 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. |
ZLeventer
left a comment
There was a problem hiding this comment.
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.
|
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:
With the custom DeleteSandbox PS :
Two steps, no disruption to existing profiles or user assignments. |



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
Implementation Details
Testing