Skip to content
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

Custom DevSkim rules (on yaml files) not showing in VSCode custom tab #659

Open
JaneX8 opened this issue Dec 5, 2024 · 18 comments
Open
Labels

Comments

@JaneX8
Copy link

JaneX8 commented Dec 5, 2024

Describe the bug
I don't manage to get DevSkim custom rules to show up in the Problems Tab in VSCode. I wrote a json file containing:

To Reproduce

[{
    "id": "TestRule",
    "name": "Basic Test Rule",
    "description": "A simple test rule.",
    "applies_to": ["yaml"],
    "tags": ["test"],
    "patterns": [{
        "pattern": "redirects",
        "type": "regex",
        "message": "Detected redirects."
    }]
}]

When I run:

devskim analyze -I "C:\pathToTestFolder" -r "C:\pathToCustomDevSkimRules"

The output shows a match on my custom rule. However when I simply open the yaml file in VSCode nothing is shown in the Problems tab.

I also added this:

    "MS-CST-E.vscode-devskim.rules.customRulesPaths": [
        "C:\\pathToCustomDevSkimRules"
    ],
    "MS-CST-E.vscode-devskim.logging.level": "debug",

Inside C:\Users\username\AppData\Roaming\VSCodium\User\settings.json. Other DevSkim rules do show up in the Problems tab.

Expected behavior
My custom rules to show up in the Problems tab in VSCode.

Versions(please complete the following information):

  • OS: Windows 11
  • Devskim Version: devskim 1.0.44+10b85ce690

Other

I saw #300 and my file extensions are actually yaml.

@JaneX8 JaneX8 added the bug label Dec 5, 2024
@JaneX8
Copy link
Author

JaneX8 commented Dec 5, 2024

Ok, I just wasted an hour debugging what was wrong. And despite restarting VSCodium and disabling and reenabling the DevSkim extensions it wasn't enough. What did the trick was

What did fix it in VSCodium/VSCode: I did Ctrl+Shift+P (or Cmd+Shift+P on macOS), then type Reload Window and hit Enter.

@JaneX8 JaneX8 closed this as completed Dec 5, 2024
@JaneX8
Copy link
Author

JaneX8 commented Dec 5, 2024

The question remains: How do I properly and quickly debug my rules? Who lints the linter? Is there a JSONSchema definition for DevSkim rules? Can I have autocomplete/syntax checking on my DevSkim rules in VSCode? I now have to constantly reload the window after every change or run the command line option and find a tracelog of a failed json parse:

Unhandled exception. System.Text.Json.JsonException: The JSON value could not be converted to Microsoft.DevSkim.FixType. Path: ...

@JaneX8 JaneX8 reopened this Dec 5, 2024
@JaneX8
Copy link
Author

JaneX8 commented Dec 5, 2024

Ok, only reloading the whole VSCode window after every change makes it work. This is a bug, it should work automatically.

@gfs
Copy link
Contributor

gfs commented Dec 5, 2024

The DevSkim CLI has a verify command that does some checks for consistency/correctness, but there isn't an entrypoint to that in the VsCode extension at this point.

The VsCode extension uses a Language Server model, so the VsCode front end launches and then talks to a C# language server that is running the C# DevSkim engine. I think what you may be encountering with it working after a reload is that when the language server relaunches it will load the specified rules from disc with your options - but while VsCode is already running any changes to those files will be reflected on disc but are not hot reloaded into the language server.

@JaneX8
Copy link
Author

JaneX8 commented Dec 5, 2024

I think that VSCode is the main reason I'm going crazy with DevSkim now because I'm trying to write custom rules and also using verify and analyze CLI and they produce different outputs (as expected) while not showing in VSCode problems tab, even after reloading the window. That's a pity because VSCode is the main reason I want to use DevSkim. I think it makes a very powerful combination if custom rules are easily writable and testable.

Also, it would be nice if we can figure out a way to reload that Language Server once changes are detected in the Custom Rules folders.

Here is an unexplained example that works in CLI but not showing in VSCode.

test.yaml

dfds:
    xyz:
      - kqe: afa
        asf:
          - 1801
    abc:
      - asd: something
        test:
          - "anything1"
          - "XXanything2"
          - "XXanything3"

example_yaml_rule.json

[{
    "name": "NestedRegexExtractorRule",
    "id": "AMLPath.NestedRegexExtractor",
    "description": "Extract regex patterns dynamically from the nested requests section of the YAML file.",
    "severity": "critical",
    "tags": ["test"],
    "patterns": [{
        "pattern": ".*",
        "ymlpaths": [
            "/dfds/abc/*/test/*"
        ],
        "type": "regex",
        "scopes": [
            "code"
        ],
        "modifiers": [
            "i"
        ],
        "confidence": "high"
    }]
}]
devskim analyze -I ".\test.yaml" -r ".\example_yaml_rule.json"
{"$schema":"https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.6.json","version":"2.1.0","runs":[{"tool":{"driver":{"name":"devskim","fullName":"Microsoft DevSkim Command Line Interface","version":"1.0.44+10b85ce690","informationUri":"https://github.com/microsoft/DevSkim/","rules":[{"id":"AMLPath.NestedRegexExtractor","name":"Nestedregexextractorrule","fullDescription":{"text":"NestedRegexExtractorRule: Extract regex patterns dynamically from the nested requests section of the YAML file."},"help":{"text":"Extract regex patterns dynamically from the nested requests section of the YAML file.","markdown":"Visit [https://github.com/Microsoft/DevSkim/blob/main/guidance/](https://github.com/Microsoft/DevSkim/blob/main/guidance/) for guidance on this issue."},"shortDescription":{"text":"Extract regex patterns dynamically from the nested requests section of the YAML file."},"defaultConfiguration":{"level":"error"},"helpUri":"https://github.com/Microsoft/DevSkim/blob/main/guidance/","properties":{"precision":"","problem.severity":"error","DevSkimSeverity":"Critical","DevSkimConfidence":"Unspecified"}}]}},"versionControlProvenance":[{"repositoryUri":"https://github.com/Rebalance3603/scanning-tool.git","revisionId":"2deea1a5840e3e0303253ec1680e9dd980948a3d","branch":"main"}],"results":[{"ruleId":"AMLPath.NestedRegexExtractor","level":"error","message":{"text":"NestedRegexExtractorRule"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"test.yaml"},"region":{"charOffset":193,"snippet":{"text":"","rendered":{"markdown":"``"}},"sourceLanguage":"yaml"}}}],"properties":{"tags":["test"],"DevSkimSeverity":"Critical","DevSkimConfidence":"Unspecified"}},{"ruleId":"AMLPath.NestedRegexExtractor","level":"error","message":{"text":"NestedRegexExtractorRule"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"test.yaml"},"region":{"startLine":11,"startColumn":12,"charOffset":180,"charLength":13,"snippet":{"text":"\"XXanything3\"","rendered":{"text":"\"XXanything3\"","markdown":"`\"XXanything3\"`"}},"sourceLanguage":"yaml"}}}],"properties":{"tags":["test"],"DevSkimSeverity":"Critical","DevSkimConfidence":"Unspecified"}},{"ruleId":"AMLPath.NestedRegexExtractor","level":"error","message":{"text":"NestedRegexExtractorRule"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"test.yaml"},"region":{"startLine":10,"startColumn":25,"endLine":10,"endColumn":25,"charOffset":166,"snippet":{"text":"","rendered":{"markdown":"``"}},"sourceLanguage":"yaml"}}}],"properties":{"tags":["test"],"DevSkimSeverity":"Critical","DevSkimConfidence":"Unspecified"}},{"ruleId":"AMLPath.NestedRegexExtractor","level":"error","message":{"text":"NestedRegexExtractorRule"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"test.yaml"},"region":{"startLine":10,"startColumn":12,"endLine":10,"endColumn":25,"charOffset":153,"charLength":13,"snippet":{"text":"\"XXanything2\"","rendered":{"text":"\"XXanything2\"","markdown":"`\"XXanything2\"`"}},"sourceLanguage":"yaml"}}}],"properties":{"tags":["test"],"DevSkimSeverity":"Critical","DevSkimConfidence":"Unspecified"}},{"ruleId":"AMLPath.NestedRegexExtractor","level":"error","message":{"text":"NestedRegexExtractorRule"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"test.yaml"},"region":{"startLine":9,"startColumn":23,"endLine":9,"endColumn":23,"charOffset":139,"snippet":{"text":"","rendered":{"markdown":"``"}},"sourceLanguage":"yaml"}}}],"properties":{"tags":["test"],"DevSkimSeverity":"Critical","DevSkimConfidence":"Unspecified"}},{"ruleId":"AMLPath.NestedRegexExtractor","level":"error","message":{"text":"NestedRegexExtractorRule"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"test.yaml"},"region":{"startLine":9,"startColumn":12,"endLine":9,"endColumn":23,"charOffset":128,"charLength":11,"snippet":{"text":"\"anything1\"","rendered":{"text":"\"anything1\"","markdown":"`\"anything1\"`"}},"sourceLanguage":"yaml"}}}],"properties":{"tags":["test"],"DevSkimSeverity":"Critical","DevSkimConfidence":"Unspecified"}}],"columnKind":"utf16CodeUnits"}]}

Nothing is shown in the Problems tab. Other DevSkim rules work and do show there.

Image

I didn't see anything worth mentioning under the output tab of DevSkim VS Code Client.

@gfs
Copy link
Contributor

gfs commented Dec 5, 2024

Sorry! I understand the frustration. I can definitely note this as a feature request to add hot-reload when custom rules are changed - as a workaround you can cause the language server to reload the rules from disk by changing a setting, you however, then also need to trigger a new analysis on the file - the trigger is scan is event driven from VS Code itself - so that should be if the file is opened or changed, so even if we could hot reload rules, we'd need something to trigger a rescan.

@gfs
Copy link
Contributor

gfs commented Dec 5, 2024

FWIW, the example rule I shared in the other thread does seem to be working in VS Code. Resharing here:

[{
    "name": "YamlPathValidate",
    "id": "YmlPath",
    "tags": ["test"],
    "description": "values under a key named test that start with XX",
    "severity": "critical",
    "patterns": [
      {
        "pattern":"XX\\w*",
        "ymlpaths" : ["/**/test/*"],
        "type": "regex",
        "scopes": [
          "code"
        ],
        "modifiers": [
          "i"
        ],
        "confidence": "high"
      }
    ]
  }]

Image

@JaneX8
Copy link
Author

JaneX8 commented Dec 5, 2024

I can confirm that one works indeed.

@JaneX8
Copy link
Author

JaneX8 commented Dec 5, 2024

I'm having the suspicion that regex patterns are not behaving the same on CLI and VSCode, or maybe it has to do with the way it is escaped and passed through the language server. The combination of ymlpath with negative lookaheads in regexes for example are working in CLI but not showing in VSC. As do some other patterns like the simple .* that I showed above.

@gfs
Copy link
Contributor

gfs commented Dec 5, 2024

I tested with the rule you were testing above and it seems to be working for me in VS Code as well.

Image

@JaneX8
Copy link
Author

JaneX8 commented Dec 5, 2024

The VsCode extension uses a Language Server model, so the VsCode front end launches and then talks to a C# language server that is running the C# DevSkim engine. I think what you may be encountering with it working after a reload is that when the language server relaunches it will load the specified rules from disc with your options - but while VsCode is already running any changes to those files will be reflected on disc but are not hot reloaded into the language server.

Probably this. I just experienced that despite reloading the window, a previous rule kept working and the new one did not yet. I need to find a way to either manually reload that language server, and then preferably get the extension to do hot reloading at some point.

Possibly related:

@gfs
Copy link
Contributor

gfs commented Dec 5, 2024

Thanks for the references. I believe we are using the language client library mentioned in 76405, I'll try to take a look and see how this could fit in. The simplest solution might be to add an action to the command palate in vscode to trigger a reload.

@JaneX8
Copy link
Author

JaneX8 commented Dec 11, 2024

I'm tired of reloading my window every few minutes when I'm developing DevSkim rules and can't hot-reload. I'm diving into this rabbit hole and find several other GH projects for VS code extensions having similar issues with the Problems tab.

Also not being able to programmatically disable and enable the DevSkim extension or restarting it doesn't help either. I tried experimenting force closing the subprocess dotnet.exe and it will be reopened automatically, the problems in the DevSkim Problems-tab disappear but don't come back, I guess an association problem. I'm testing with VSCodium but VSCode should be the same. This is absolute overkill but the main process VSCode contains another process VSCode which contains the dotnet.exe (DevSkim). When I kill that middle process (which I think is the whole extensionhost), reloads the problems tab entirely, with new DevSkim rules.

I'm gonna write a DevSkim custom rules directory watcher that does this on any custom DevSkim rule change and use it when developing and testing DevSkim rules. Note that is is a very quick and dirty workaround for my problem for now. And not a recommended solution. It works for my purpose. Tested with one VSCodium window open, with many extensions where in my case only DevSkim uses dotnet.exe.

I wrote the following small powershell script that I'm going to use in a directory watcher when writing custom DevSkim rules:

$topVSCodium = (Get-Process -Name VSCodium | Sort-Object StartTime | Select-Object -First 1)
$dotnet = (Get-CimInstance Win32_Process | Where-Object { $_.Name -eq "dotnet.exe" })
if ($dotnet) {
    $parent = (Get-CimInstance Win32_Process | Where-Object { $_.ProcessId -eq $dotnet.ParentProcessId -and $_.Name -eq "VSCodium.exe" })
    if ($parent -and $parent.ProcessId -ne $topVSCodium.Id) {
        Stop-Process -Id $parent.ProcessId -Force
        Write-Host "Killed VSCodium parent process of dotnet.exe (PID: $($parent.ProcessId))."
    } else {
        Write-Host "Parent process is the top-level VSCodium.exe; not killing it."
    }
} else {
    Write-Host "No dotnet.exe process found under VSCodium."
}

From sysinternals process explorer, the selected parent of dotnet.exe seems to be the extensionshost process:

Image

I'm not sure if this is something that we could even fix in a VSCode extension, it seems to me it's a VSCode core issue but I hope I'm wrong. Still remains: hot-reloading would make writing customs rules much easier and faster.

@gfs
Copy link
Contributor

gfs commented Dec 11, 2024

I think this unfortunately comes down to a VSCode architecture limitation - even if you restart the extension process the files themselves needs to be processed again to get new issues, which gets triggered by events fired by vscode like file change. The easiest way to trigger this, in my experience, is to add a character to a file you want rescanned and then delete it. I think it is possible to add a manual action to explicitly kick off a scan of all files in the workspace - but this is not implemented in the DevSkim extension, and I'm not sure how trivial it would be to add. I personally develop and debug rules using the command line version of devskim.

@JaneX8
Copy link
Author

JaneX8 commented Dec 11, 2024

I tried to find another way to programmatically reload or force a trigger in other ways like adding a file to the workspace, deleting a file to the workspace, editing a temporary file in the workspace, changing timestamps of existing files, none trigger a reload. I couldn't find any other way to achieve this which is why I found the right process and killed it. Not a solution at all, very dirty workaround..

@JaneX8
Copy link
Author

JaneX8 commented Dec 16, 2024

@gfs I think you are in a better position to answer microsoft/vscode#235799 (comment). Could you maybe?

@gfs
Copy link
Contributor

gfs commented Jan 6, 2025

The problem is not that we cannot restart the language server, as mentioned in my earlier comment restarting the server is not actually needed here - the settings themselves including the ruleset being used can be changed dynamically when the settings are updated without a restart of the language server.

The issue you are experiencing is that we scan files based on triggers from VS Code when those files are opened or changed - if you change the ruleset dynamically, the files that are to be scanned have not changed and thus will not be rescanned until they are changed.

I don't think this is something that VS Code can really "fix", the current workaround is to make a change in the file you want to scan. To fix this in DevSKim itself, we would need to, as mentioned above, add a new manual action to the command palette in VS Code that would scan either the current active file/all files in the workspace. However, we do not currently interface with files in VS Code in any similar way so its non-trivial to add this, and would add a fair bit of complexity to the Language Client which has been kept intentionally thin thus far. This also would create a feature differential between VS Code and the VS 2022 extensions - though that's less of a concern.

To my knowledge, what would be required to implement the solution I propose above adding the new command would require:

  1. the coding/configuration for new command trigger to populate it in the command palette (https://code.visualstudio.com/api/extension-guides/command)
  2. Inside the logic for the triggered command it would need to call the Workspace api (https://code.visualstudio.com/api/references/vscode-api#workspace) to enumerate the open files and send the content to the language server to scan
  3. Likely a new custom JSONRPC command definition and handler to handle those ad-hoc scans (for example, this one is used to update the code fixes when new diagnostics are returned:
    [JsonRpcMethod(DevSkimMessages.CodeFixMapping)]
    public async Task CodeFixMappingEventAsync(JToken jToken)
    {
    await Task.Run(() =>
    {
    CodeFixMapping mapping = jToken.ToObject<CodeFixMapping>();
    if (mapping is { })
    {
    StaticData.FileToCodeFixMap.AddOrUpdate(mapping.fileName,
    // Add New Nested Dictionary
    (Uri _) => new (new Dictionary<int, ConcurrentDictionary<CodeFixMapping, bool>>
    { { mapping.version ?? -1, new (new Dictionary<CodeFixMapping, bool>()
    { {mapping, true } }) } }),
    // Update Nested Dictionary
    (key, oldValue) =>
    {
    oldValue.AddOrUpdate(mapping.version ?? -1,
    // Add new Set of mappings
    (int _) =>
    {
    var addedMapping = new ConcurrentDictionary<CodeFixMapping, bool>();
    addedMapping.TryAdd(mapping, true);
    return addedMapping;
    },
    // Update Set of CodeFixMappings
    (versionKey, oldSet) => { oldSet.TryAdd(mapping, true); return oldSet; });
    return oldValue;
    });
    }
    });
    }
    )
  4. Scanning the files in the language server and responding with diagnostics and codefixes

While 4 should be easy to hook up and 3 is something we've done before, 1 and 2 are entirely new APIs and features we don't implement today and I'm not entirely convinced that its worth the additional effort vs the workaround of adding and removing a space from the file you want to trigger a rescan on, particularly since this is a feature specifically for debugging rules and not general usage.

@gfs
Copy link
Contributor

gfs commented Jan 8, 2025

I opened #674 to track the feature request to auto-rescan on settinngs change/add a new command to manually scan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants