Skip to content

Update check for use of GlideRecordSecure on Client Callable Script Includes #85

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

Closed
niamccash opened this issue Oct 11, 2023 · 9 comments

Comments

@niamccash
Copy link
Contributor

Create new Linter Check type Instance Scan for use of GlideRecord Secure on Client Callable Script Includes

As per https://docs.servicenow.com/bundle/utah-api-reference/page/script/server-scripting/concept/c_ScriptIncludes.html#title_client-callable-script-includes

image

Note: there is already an existing Table Check for this. This issue is for a Linter type check.

@MartinStoyanoff
Copy link
Contributor

Hi @niamccash I just had a chat with a friend who is deep into the instance scan and he recommended to do this with Scritp only check, rather than Linter check.
His arguments are:

  • Linter Check is not specifically Script Include, it will run on all scripts.
  • we can add a condition to check only the script include table, but it will still run all of the time, which is bad practioce.

let me know if you still insist to have a Linter check or I can take it over and create a Script only check.
Thank you

@niamccash
Copy link
Contributor Author

@MartinStoyanoff That's good to know. Thanks for sharing.
Could the Script only check catch instances where GlideRecord use is commented out?

I like Linter checks because it does check for actual code use and ignores things like

// var gr = new GlideRecord('incident');

Where devs may have commented out old code instead of removing.

@aman2519
Copy link
Contributor

Hi @niamccash
Should I start with the linter check then?

@niamccash
Copy link
Contributor Author

Hi @aman2519.

I'm waiting for @MartinStoyanoff's input but I don't see the harm in having multiple checks. If you want to create the Linter check and submit a PR, I will happily accept.

As there will be multiple check types that scan for the same thing, it would be nice to have some comments in the code to help explain how the Linter check is different than the Table check and a Script Only check.

As an added challenge, see if you can add a condition to your Linter check to only check the script include table.

@MartinStoyanoff
Copy link
Contributor

@aman2519 you can take it and implement the linter check.
@niamccash to your question, it will check commented code as well. What I pointed out in my previous comment is an input from Mark Roethof. I am not an instance scan expert at all.

@markroethof
Copy link
Contributor

@MartinStoyanoff @niamccash For checking commented code, there are some out-of-the-box checks which look to be something like that... I haven't validated it myself. See for example:
https://your-instance.service-now.com/nav_to.do?uri=scan_column_type_check.do?sys_id=dfd6ccb8db796510dd95b7e8f49619eb

@markroethof
Copy link
Contributor

@niamccash Linter Check could still work fine if its for a specific table, then you need to start the check with something like engine.current.getTableName() == 'lalala' else return. Downside... the Linter Check will run for several minutes, while the Script Only Check or Table Check will run only for a few seconds.

@Lacah
Copy link
Contributor

Lacah commented Sep 29, 2024

@niamccash wanna keep this issue open? What are the latest requirements? :)

@niamccash
Copy link
Contributor Author

@Lacah The table check is done but there's still opportunity for someone to do a Linter check if they wanted to. But perhaps a new Issue would be easier and more appropriate, so I'll close this one for now.

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

No branches or pull requests

5 participants