From a48424d79369fc2ff0614dbcdfb06f3bacdf8e82 Mon Sep 17 00:00:00 2001 From: Brahim LAISSAOUI Date: Sun, 14 Dec 2025 11:59:25 +0100 Subject: [PATCH 1/3] blaissao : Enforce 'DeleteSandbox' Permission Set requirement for Sandbox Deletion --- messages/delete_sandbox.md | 8 ++++ src/commands/org/delete/sandbox.ts | 63 ++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/messages/delete_sandbox.md b/messages/delete_sandbox.md index bf24b3f7a..e6efa9864 100644 --- a/messages/delete_sandbox.md +++ b/messages/delete_sandbox.md @@ -54,3 +54,11 @@ Ensure the CLI has authenticated with the sandbox's production org. # error.missingUsername Unable to determine the username of the org to delete. Specify the username with the --target-org | -o flag. + +# error.insufficientPermissions + +You do not have the required permission to delete this sandbox: %s. Contact your administrator to assign you the "DeleteSandbox" PermissionSet. + +# warning.couldNotVerifyPermissions + +Could not verify permissions in sandbox: %s. The delete operation will proceed, but may fail if you do not have the required permissions. diff --git a/src/commands/org/delete/sandbox.ts b/src/commands/org/delete/sandbox.ts index 1646a15d5..9069d2a8e 100644 --- a/src/commands/org/delete/sandbox.ts +++ b/src/commands/org/delete/sandbox.ts @@ -61,6 +61,29 @@ export default class DeleteSandbox extends SfCommand { throw messages.createError('error.unknownSandbox', [username]); } + // Check if user has the DeleteSandbox PermissionSet in the sandbox + try { + const sandboxOrg = await Org.create({ aliasOrUsername: username }); + const hasDeleteSandboxPermission = await this.hasPermission(sandboxOrg, 'DeleteSandbox'); + this.debug('hasDeleteSandboxPermission %s ', hasDeleteSandboxPermission); + if (!hasDeleteSandboxPermission) { + throw messages.createError('error.insufficientPermissions', [username]); + } + } catch (error) { + // If it's a permission error we created, re-throw it + if (error instanceof SfError) { + const errorMessage = error.message || ''; + if (errorMessage.includes('required permission') || errorMessage.includes('DeleteSandbox')) { + throw error; + } + } + // For other errors (e.g., org connection issues), log a warning but continue + // The actual delete operation will fail if permissions are truly insufficient + if (error instanceof Error) { + this.warn(messages.getMessage('warning.couldNotVerifyPermissions', [username])); + } + } + if (flags['no-prompt'] || (await this.confirm({ message: messages.getMessage('prompt.confirm', [username]) }))) { try { const org = await Org.create({ aliasOrUsername: username }); @@ -82,4 +105,44 @@ export default class DeleteSandbox extends SfCommand { } return { username, orgId }; } + + /** + * Checks if the current user has a PermissionSet with the specified name assigned. + * + * @param org The org to check permissions in + * @param permissionSetName The name of the PermissionSet to check (e.g., 'DeleteSandbox') + * @returns True if the user has the PermissionSet assigned, false otherwise + */ + // eslint-disable-next-line class-methods-use-this + private async hasPermission(org: Org, permissionSetName: string): Promise { + try { + const connection = org.getConnection(); + await org.refreshAuth(); + // try to get it from Identity API + const identity = await connection.identity(); + const userId = identity.user_id; + + if (!userId) { + return false; + } + + // Check if user has the PermissionSet assigned + const permissionSetAssignmentQuery = ` + SELECT Id + FROM PermissionSetAssignment + WHERE AssigneeId = '${userId.replace(/'/g, "\\'")}' + AND PermissionSet.Name = '${permissionSetName.replace(/'/g, "\\'")}' + `; + + try { + const permissionSetResult = await connection.query(permissionSetAssignmentQuery); + return permissionSetResult.totalSize > 0; + } catch { + // If query fails, return false + return false; + } + } catch { + return false; + } + } } From 4d15afbaeeb750042b44159f51e83efcc78e1f13 Mon Sep 17 00:00:00 2001 From: Brahim LAISSAOUI Date: Sat, 25 Apr 2026 13:13:24 +0200 Subject: [PATCH 2/3] blaissao : apply suggested fixs (code refactoring) --- messages/delete_sandbox.md | 11 +++- src/commands/org/delete/sandbox.ts | 95 ++++++++++++------------------ test/unit/org/delete.test.ts | 63 +++++++++++++++++++- 3 files changed, 108 insertions(+), 61 deletions(-) diff --git a/messages/delete_sandbox.md b/messages/delete_sandbox.md index e6efa9864..f367f3042 100644 --- a/messages/delete_sandbox.md +++ b/messages/delete_sandbox.md @@ -57,8 +57,13 @@ Unable to determine the username of the org to delete. Specify the username with # error.insufficientPermissions -You do not have the required permission to delete this sandbox: %s. Contact your administrator to assign you the "DeleteSandbox" PermissionSet. +You don't have the required "DeleteSandbox" Permission Set assigned in the production org to delete sandbox "%s". -# warning.couldNotVerifyPermissions +# error.insufficientPermissions.actions -Could not verify permissions in sandbox: %s. The delete operation will proceed, but may fail if you do not have the required permissions. +- Ask your administrator to create a Permission Set named "DeleteSandbox" in the production org and assign it to your user. +- Re-authenticate with the production org and try again. + +# error.insufficientAccess + +You don't have permission to delete this sandbox. Ask your Salesforce admin to grant you the "Manage Sandboxes" system permission on your profile or a permission set in the production org. diff --git a/src/commands/org/delete/sandbox.ts b/src/commands/org/delete/sandbox.ts index 9069d2a8e..5def3c5f5 100644 --- a/src/commands/org/delete/sandbox.ts +++ b/src/commands/org/delete/sandbox.ts @@ -61,43 +61,27 @@ export default class DeleteSandbox extends SfCommand { throw messages.createError('error.unknownSandbox', [username]); } - // Check if user has the DeleteSandbox PermissionSet in the sandbox - try { - const sandboxOrg = await Org.create({ aliasOrUsername: username }); - const hasDeleteSandboxPermission = await this.hasPermission(sandboxOrg, 'DeleteSandbox'); - this.debug('hasDeleteSandboxPermission %s ', hasDeleteSandboxPermission); - if (!hasDeleteSandboxPermission) { - throw messages.createError('error.insufficientPermissions', [username]); - } - } catch (error) { - // If it's a permission error we created, re-throw it - if (error instanceof SfError) { - const errorMessage = error.message || ''; - if (errorMessage.includes('required permission') || errorMessage.includes('DeleteSandbox')) { - throw error; - } - } - // For other errors (e.g., org connection issues), log a warning but continue - // The actual delete operation will fail if permissions are truly insufficient - if (error instanceof Error) { - this.warn(messages.getMessage('warning.couldNotVerifyPermissions', [username])); - } - } + await this.verifyDeletePermission(stateAggregator, orgId, username); + + const org = await Org.create({ aliasOrUsername: username }); if (flags['no-prompt'] || (await this.confirm({ message: messages.getMessage('prompt.confirm', [username]) }))) { try { - const org = await Org.create({ aliasOrUsername: username }); await org.delete(); this.logSuccess(messages.getMessage('success', [username])); } catch (e) { if (e instanceof Error && e.name === 'DomainNotFoundError') { - // the org has expired, so remote operations won't work - // let's clean up the files locally const authRemover = await AuthRemover.create(); await authRemover.removeAuth(username); this.logSuccess(messages.getMessage('success.Idempotent', [username])); } else if (e instanceof Error && e.name === 'SandboxNotFound') { this.logSuccess(messages.getMessage('success.Idempotent', [username])); + } else if ( + e instanceof Error && + 'errorCode' in e && + (e as { errorCode: string }).errorCode === 'INSUFFICIENT_ACCESS_OR_READONLY' + ) { + throw messages.createError('error.insufficientAccess'); } else { throw e; } @@ -107,42 +91,39 @@ export default class DeleteSandbox extends SfCommand { } /** - * Checks if the current user has a PermissionSet with the specified name assigned. - * - * @param org The org to check permissions in - * @param permissionSetName The name of the PermissionSet to check (e.g., 'DeleteSandbox') - * @returns True if the user has the PermissionSet assigned, false otherwise + * Verifies the current user has the 'DeleteSandbox' PermissionSet assigned + * in the production org that owns the sandbox. */ // eslint-disable-next-line class-methods-use-this - private async hasPermission(org: Org, permissionSetName: string): Promise { - try { - const connection = org.getConnection(); - await org.refreshAuth(); - // try to get it from Identity API - const identity = await connection.identity(); - const userId = identity.user_id; - - if (!userId) { - return false; - } + private async verifyDeletePermission( + stateAggregator: StateAggregator, + orgId: string, + sandboxUsername: string + ): Promise { + const sandboxConfig = await stateAggregator.sandboxes.read(orgId); + const prodOrgUsername = sandboxConfig?.prodOrgUsername; - // Check if user has the PermissionSet assigned - const permissionSetAssignmentQuery = ` - SELECT Id - FROM PermissionSetAssignment - WHERE AssigneeId = '${userId.replace(/'/g, "\\'")}' - AND PermissionSet.Name = '${permissionSetName.replace(/'/g, "\\'")}' - `; + if (!prodOrgUsername) { + return; + } - try { - const permissionSetResult = await connection.query(permissionSetAssignmentQuery); - return permissionSetResult.totalSize > 0; - } catch { - // If query fails, return false - return false; - } - } catch { - return false; + const prodOrg = await Org.create({ aliasOrUsername: prodOrgUsername }); + const connection = prodOrg.getConnection(); + await prodOrg.refreshAuth(); + + const identity = await connection.identity(); + const userId = identity.user_id; + + if (!userId) { + throw messages.createError('error.insufficientPermissions', [sandboxUsername]); + } + + const result = await connection.query( + `SELECT Id FROM PermissionSetAssignment WHERE AssigneeId = '${userId}' AND PermissionSet.Name = 'DeleteSandbox'` + ); + + if (result.totalSize === 0) { + throw messages.createError('error.insufficientPermissions', [sandboxUsername]); } } } diff --git a/test/unit/org/delete.test.ts b/test/unit/org/delete.test.ts index 1cc211d8b..081b83e61 100644 --- a/test/unit/org/delete.test.ts +++ b/test/unit/org/delete.test.ts @@ -5,7 +5,7 @@ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ -import { AuthInfo, Messages, Org, SfError } from '@salesforce/core'; +import { AuthInfo, Connection, Messages, Org, SfError } from '@salesforce/core'; import { MockTestOrgData, TestContext } from '@salesforce/core/testSetup'; import { SinonStub } from 'sinon'; import { config, expect } from 'chai'; @@ -38,6 +38,12 @@ describe('org delete', () => { }); describe('sandbox', () => { + let sandboxReadStub: SinonStub; + + beforeEach(() => { + sandboxReadStub = $$.SANDBOX.stub(SandboxAccessor.prototype, 'read').resolves(null); + }); + it('will throw an error when no org provided', async () => { await $$.stubConfig({}); try { @@ -128,6 +134,61 @@ describe('org delete', () => { JSON.stringify(sfCommandUxStubs.logSuccess.getCalls().flatMap((call) => call.args)) ).to.deep.include(sbxOrgMessages.getMessage('success.Idempotent', [testOrg.username])); }); + + it('will throw a clean SfError when the user lacks Manage Sandboxes permission', async () => { + $$.SANDBOX.stub(SandboxAccessor.prototype, 'hasFile').resolves(true); + orgDeleteStub.restore(); + const insufficientAccessError = Object.assign(new Error('INSUFFICIENT_ACCESS_OR_READONLY'), { + errorCode: 'INSUFFICIENT_ACCESS_OR_READONLY', + }); + $$.SANDBOX.stub(Org.prototype, 'delete').throws(insufficientAccessError); + try { + await DeleteSandbox.run(['--no-prompt', '--target-org', testOrg.username]); + expect.fail('should have thrown InsufficientAccessError'); + } catch (e) { + const err = e as SfError; + expect(err.name).to.equal('InsufficientAccessError'); + expect(err.message).to.equal(sbxOrgMessages.getMessage('error.insufficientAccess')); + } + }); + + describe('DeleteSandbox permission check', () => { + let identityStub: SinonStub; + let queryStub: SinonStub; + + beforeEach(() => { + $$.SANDBOX.stub(SandboxAccessor.prototype, 'hasFile').resolves(true); + $$.SANDBOX.stub(Org.prototype, 'refreshAuth').resolves(); + identityStub = $$.SANDBOX.stub(Connection.prototype, 'identity'); + queryStub = $$.SANDBOX.stub(Connection.prototype, 'query'); + }); + + it('will allow deletion when user has DeleteSandbox permission set', async () => { + sandboxReadStub.resolves({ sandboxOrgId: testOrg.orgId, prodOrgUsername: testHub.username }); + // eslint-disable-next-line camelcase + identityStub.resolves({ user_id: '005xx000001X' }); + queryStub.resolves({ totalSize: 1, records: [{ Id: '0Pa000000000001' }] }); + const res = await DeleteSandbox.run(['--no-prompt', '--target-org', testOrg.username]); + expect(sfCommandUxStubs.logSuccess.callCount).to.equal(1); + expect(res).to.deep.equal({ orgId: testOrg.orgId, username: testOrg.username }); + }); + + it('will block deletion when user lacks DeleteSandbox permission set', async () => { + sandboxReadStub.resolves({ sandboxOrgId: testOrg.orgId, prodOrgUsername: testHub.username }); + // eslint-disable-next-line camelcase + identityStub.resolves({ user_id: '005xx000001X' }); + queryStub.resolves({ totalSize: 0, records: [] }); + try { + await DeleteSandbox.run(['--no-prompt', '--target-org', testOrg.username]); + expect.fail('should have thrown InsufficientPermissionsError'); + } catch (e) { + const err = e as SfError; + expect(err.name).to.equal('InsufficientPermissionsError'); + expect(err.message).to.include(testOrg.username); + expect(err.message).to.include('DeleteSandbox'); + } + }); + }); }); describe('scratch', () => { From a7de4e5cd00dcff6651a224cc9f890ec6c72ac08 Mon Sep 17 00:00:00 2001 From: Brahim LAISSAOUI Date: Sat, 25 Apr 2026 20:44:32 +0200 Subject: [PATCH 3/3] blaissao : apply suggested fixs v2 --- messages/delete_sandbox.md | 8 ++++++ src/commands/org/delete/sandbox.ts | 2 +- test/unit/org/delete.test.ts | 39 ++++++++++++++++++------------ 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/messages/delete_sandbox.md b/messages/delete_sandbox.md index f367f3042..34e16a7ca 100644 --- a/messages/delete_sandbox.md +++ b/messages/delete_sandbox.md @@ -64,6 +64,14 @@ You don't have the required "DeleteSandbox" Permission Set assigned in the produ - Ask your administrator to create a Permission Set named "DeleteSandbox" in the production org and assign it to your user. - Re-authenticate with the production org and try again. +# error.missingProdOrg + +Unable to verify delete permissions for sandbox "%s" because the production org could not be resolved from the sandbox configuration. + +# error.missingProdOrg.actions + +- Ensure the production org that owns this sandbox is authenticated with the CLI using `sf org login web`. + # error.insufficientAccess You don't have permission to delete this sandbox. Ask your Salesforce admin to grant you the "Manage Sandboxes" system permission on your profile or a permission set in the production org. diff --git a/src/commands/org/delete/sandbox.ts b/src/commands/org/delete/sandbox.ts index 5def3c5f5..5c6e28075 100644 --- a/src/commands/org/delete/sandbox.ts +++ b/src/commands/org/delete/sandbox.ts @@ -104,7 +104,7 @@ export default class DeleteSandbox extends SfCommand { const prodOrgUsername = sandboxConfig?.prodOrgUsername; if (!prodOrgUsername) { - return; + throw messages.createError('error.missingProdOrg', [sandboxUsername]); } const prodOrg = await Org.create({ aliasOrUsername: prodOrgUsername }); diff --git a/test/unit/org/delete.test.ts b/test/unit/org/delete.test.ts index 081b83e61..3efc47d6f 100644 --- a/test/unit/org/delete.test.ts +++ b/test/unit/org/delete.test.ts @@ -41,7 +41,15 @@ describe('org delete', () => { let sandboxReadStub: SinonStub; beforeEach(() => { - sandboxReadStub = $$.SANDBOX.stub(SandboxAccessor.prototype, 'read').resolves(null); + sandboxReadStub = $$.SANDBOX.stub(SandboxAccessor.prototype, 'read').resolves({ + sandboxOrgId: testOrg.orgId, + prodOrgUsername: testHub.username, + }); + $$.SANDBOX.stub(Org.prototype, 'refreshAuth').resolves(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument, camelcase + $$.SANDBOX.stub(Connection.prototype, 'identity').resolves({ user_id: '005xx000001X' } as any); + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument + $$.SANDBOX.stub(Connection.prototype, 'query').resolves({ totalSize: 1, records: [{ Id: '0Pa000000000001' }] } as any); }); it('will throw an error when no org provided', async () => { @@ -153,31 +161,20 @@ describe('org delete', () => { }); describe('DeleteSandbox permission check', () => { - let identityStub: SinonStub; - let queryStub: SinonStub; - beforeEach(() => { $$.SANDBOX.stub(SandboxAccessor.prototype, 'hasFile').resolves(true); - $$.SANDBOX.stub(Org.prototype, 'refreshAuth').resolves(); - identityStub = $$.SANDBOX.stub(Connection.prototype, 'identity'); - queryStub = $$.SANDBOX.stub(Connection.prototype, 'query'); }); it('will allow deletion when user has DeleteSandbox permission set', async () => { - sandboxReadStub.resolves({ sandboxOrgId: testOrg.orgId, prodOrgUsername: testHub.username }); - // eslint-disable-next-line camelcase - identityStub.resolves({ user_id: '005xx000001X' }); - queryStub.resolves({ totalSize: 1, records: [{ Id: '0Pa000000000001' }] }); const res = await DeleteSandbox.run(['--no-prompt', '--target-org', testOrg.username]); expect(sfCommandUxStubs.logSuccess.callCount).to.equal(1); expect(res).to.deep.equal({ orgId: testOrg.orgId, username: testOrg.username }); }); it('will block deletion when user lacks DeleteSandbox permission set', async () => { - sandboxReadStub.resolves({ sandboxOrgId: testOrg.orgId, prodOrgUsername: testHub.username }); - // eslint-disable-next-line camelcase - identityStub.resolves({ user_id: '005xx000001X' }); - queryStub.resolves({ totalSize: 0, records: [] }); + (Connection.prototype.query as SinonStub).restore(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument + $$.SANDBOX.stub(Connection.prototype, 'query').resolves({ totalSize: 0, records: [] } as any); try { await DeleteSandbox.run(['--no-prompt', '--target-org', testOrg.username]); expect.fail('should have thrown InsufficientPermissionsError'); @@ -188,6 +185,18 @@ describe('org delete', () => { expect(err.message).to.include('DeleteSandbox'); } }); + + it('will throw when prodOrgUsername is missing from sandbox config', async () => { + sandboxReadStub.resolves({ sandboxOrgId: testOrg.orgId }); + try { + await DeleteSandbox.run(['--no-prompt', '--target-org', testOrg.username]); + expect.fail('should have thrown MissingProdOrgError'); + } catch (e) { + const err = e as SfError; + expect(err.name).to.equal('MissingProdOrgError'); + expect(err.message).to.include(testOrg.username); + } + }); }); });