Skip to content

Prevent 'Script-thrown exception' being the exception message #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

FishOfPrey
Copy link
Contributor

The SFDCAccessControlException wasn't setting the exception message. This resulted in the message being 'Script-thrown exception'.
Explicitly call the Exception constructor to set the message with useful details.

…This resulted in the message being 'Script-thrown exception'.

Explicitly call the Exception constructor to set the message.
@capeterson
Copy link
Collaborator

Internal product security guidelines frown on the disclosure of what fields the running user doesn't have access to. Since this library is security-endorsed I'll need to take a closer look at this and we might need to limit what we add to the actual exception string to avoid accidental disclosure of schema details.

@FishOfPrey
Copy link
Contributor Author

FishOfPrey commented Oct 13, 2019

Would it be more acceptable to have the message not include the object type and field name?

So just eText + ' ' + eType + ' ' + eReason.
eType and eReason are both enums defined of the exception class.

Current eText values defined in SFDCAccessController are:

  • 'Field not found
  • 'Access Violation'
  • 'Record does not exist or not shared with current user'
  • 'At least some record do not exist or are not shared with current user'

I see a typo in that last hard-coded message around "record(s)".


There is an existing getMessage() override that just returns eText. It has the comments about avoiding "script-thrown exception". Maybe at a minimum we could just use eText then.

@capeterson
Copy link
Collaborator

Would it be more acceptable to have the message not include the object type and field name?
Yes.

The most liberal messages I can merge would be allowing to reference the SObject type's name only if the user has read access at the object level. Removing the object/field name from the message is fine. System.debugging it when the exception is constructed is also fine.

Making it part of other attributes of the exception is fine, but the message shown to end users when unhandled exceptions happen needs to not leak schema details.

Making that information available programatically is fine. Just can't be in the unhandled exception default path.

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