Update check for use of GlideRecordSecure on Client Callable Script Includes
niamccash opened this issue · 9 comments
Create new Linter Check type Instance Scan for use of GlideRecord Secure on Client Callable Script Includes
Note: there is already an existing Table Check for this. This issue is for a Linter type check.
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
@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.
Hi @niamccash
Should I start with the linter check then?
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.
@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.
@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
@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.
@niamccash wanna keep this issue open? What are the latest requirements? :)