Improving rule accuracy #523
Conversation
| "rule_info": "DS180000.md", | ||
| "patterns": [ | ||
| { | ||
| "pattern": "setWebContentsDebugginEnabled(true)", |
There was a problem hiding this comment.
Typo? Missing the g for on debuggin.
There was a problem hiding this comment.
Oh whoops, good catch
There was a problem hiding this comment.
Overall looks like good improvements. I think we have the ability to have a couple of these have automatic fixes - particularly the ones doing the xml parsing for android.
Other comments are about how many variations of c# instantiation the XSLT query can detect.
| "name": "Android debug is enabled.", | ||
| "id": "DS180000", | ||
| "description": "The android:debuggable element is set to true, which should be disabled for release builds.", | ||
| "recommendation": "", |
There was a problem hiding this comment.
| "recommendation": "", | |
| "recommendation": "Set android:debuggable to false for release builds.", |
| "name": "Android debug is enabled.", | ||
| "id": "DS180001", | ||
| "description": "The setWebContentsDebuggingEnabled element is set to true, which should be disabled for release builds.", | ||
| "recommendation": "", |
There was a problem hiding this comment.
| "recommendation": "", | |
| "recommendation": "Set setWebContentsDebuggingEnabled to false for release builds.", | |
| "name": "Android StrictMode is enabled.", | ||
| "id": "DS180002", | ||
| "description": "StrictMode is detected, which is useful for developers but should be disabled for release builds.", | ||
| "recommendation": "", |
There was a problem hiding this comment.
| "recommendation": "", | |
| "recommendation": "Disable StrictMode for release builds.", | |
| "name": "XSLT Scripting Enabled", | ||
| "id": "DS132781", | ||
| "description": "XSLT Scripting is a feature that should only be enabled if script support is necessary and you are certain this is used in a trusted environment.", | ||
| "recommendation": "", |
There was a problem hiding this comment.
| "recommendation": "", | |
| "recommendation": "Disable XSLT scripting.", |
| } | ||
| ], | ||
| "must-match": [ | ||
| "XsltSettings n = new XsltSettings();\n n.EnableScript=true" |
There was a problem hiding this comment.
Does this detect
XsltSettings n = new XsltSettings(){ EnableScript = true };or
var settings = new XsltSettings(){ EnableScript = true };or
XsltSettings n = new(){ EnableScript = true };
|
@atrompler added references to the devskim issues I think this PR aims to resolve. Can you double check that list is accurate? |
| "modifiers" : ["i"] | ||
| } | ||
| ], | ||
| "fix_its": [ |
There was a problem hiding this comment.
I believe these fix it's need to specify a search-in of finding-only.
In particular updates App inspector rules engine to support xpath namespaces.
|
Test failures seem odd. Issues with files not found. I suspect just a transient pipeline issue and worth just rerunning them first. |
The test failures revealed an issue in Recursive Extractor pulled into AI. Now fixed. |
Fix #472
Fix #452
Fix #544
Fix #546
Related:
#339
#231