feat(windows): ide support#29
Conversation
Signed-off-by: Swarit Pandey <swarit@stepsecurity.io>
Signed-off-by: Swarit Pandey <swarit@stepsecurity.io>
On Windows, Go's os/exec escapes double quotes with backslashes when passing arguments to cmd.exe, but cmd.exe treats \ as a path separator. This caused every "cd <path> && npm ls" command to fail with "The filename, directory name, or volume label syntax is incorrect." Result: all project-level NPM packages were missing from telemetry on Windows (only global packages were collected). Fix: Add RunInDir to the Executor interface which sets exec.Cmd.Dir directly, bypassing the shell entirely. Update scanProject() and scanYarnGlobal() to use RunInDir instead of shell cd.
b8b48e0 to
bd373c5
Compare
There was a problem hiding this comment.
Pull request overview
Adds Windows-focused IDE support by expanding IDE detection to include JetBrains + Eclipse and introducing JetBrains plugin scanning, along with an executor enhancement to run commands in a specific working directory (avoiding Windows cd/quoting pitfalls).
Changes:
- Add JetBrains IDE + Eclipse IDE detection on macOS/Windows (including version extraction via
product-info.json/.eclipseproduct). - Add JetBrains plugin detection and include results in extension/plugin reporting.
- Add
Executor.RunInDirand refactor Node scanning to use it instead of shellcdchaining.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/telemetry/telemetry.go | Appends JetBrains plugin results into the collected IDE extensions list for telemetry payloads. |
| internal/scan/scanner.go | Appends JetBrains plugin results into the collected IDE extensions list for community scans. |
| internal/output/pretty.go | Adds display names for newly detected IDE types (JetBrains + Eclipse). |
| internal/output/pretty_test.go | Extends coverage for ideDisplayName mapping with JetBrains + Eclipse cases. |
| internal/executor/executor.go | Adds RunInDir to the Executor interface and implements it in the real executor. |
| internal/executor/mock.go | Implements RunInDir on the mock executor to keep tests working. |
| internal/detector/shellcmd.go | Removes the now-unused runShellCmd helper, leaving quoting utilities. |
| internal/detector/nodescan.go | Switches Yarn global/project execution to RunInDir and introduces runInDir helper for root delegation path. |
| internal/detector/nodescan_test.go | Updates some Windows NodeScanner tests to match the new RunInDir call shape. |
| internal/detector/jetbrains.go | New detector for JetBrains user-installed plugins using product-info.json + plugins directory scanning. |
| internal/detector/jetbrains_test.go | Adds comprehensive tests for JetBrains plugin detection across macOS/Windows and edge cases. |
| internal/detector/ide.go | Adds JetBrains/Eclipse IDE definitions, Windows glob path support, and new version extraction helpers. |
| internal/detector/ide_test.go | Adds tests for JetBrains/Eclipse detection and helper function tests for new version extractors. |
| SCAN_COVERAGE.md | Documents new IDE detection and JetBrains plugin scanning coverage. |
| CHANGELOG.md | Documents new IDE/plugin detection features under Unreleased. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // resolveInstallDir resolves a Windows path to an install directory. | ||
| // Supports glob patterns (e.g., "C:\Program Files\JetBrains\GoLand *") | ||
| // for IDEs that embed version numbers in folder names. | ||
| // When multiple matches exist, returns the last (newest by lexicographic order). | ||
| func (d *IDEDetector) resolveInstallDir(resolved string) (string, bool) { |
There was a problem hiding this comment.
resolveInstallDir selects the “newest” match by lexicographic sort, which can pick the wrong installation when version components have different digit counts (e.g., 2024.9 sorts after 2024.10). Consider selecting by a parsed version (ideally by reading each match’s product-info.json version and comparing) or by directory mtime instead of plain string order.
| cmd := "cd " + platformShellQuote(s.exec, dir) + " && " + name | ||
| for _, a := range args { | ||
| cmd += " " + a | ||
| } |
There was a problem hiding this comment.
In the root-delegation path, runInDir constructs a shell command by concatenating name/args without shell-quoting. Any argument containing spaces or shell metacharacters will be mis-parsed (and could be abused if any args ever include user-controlled input). Please quote/escape name and each arg (e.g., using platformShellQuote) or avoid bash -c entirely by extending the executor to run a command as a user with a working directory.
| start := time.Now() | ||
| shellCmd := "cd " + platformShellQuote(s.exec, globalDir) + " && yarn list --json --depth=0" | ||
| stdout, stderr, exitCode, _ := s.runShellCmd(ctx, 60*time.Second, shellCmd) | ||
| // Run directly in the global dir instead of shell cd (avoids Windows quoting issues). | ||
| stdout, stderr, exitCode, _ := s.runInDir(ctx, globalDir, 60*time.Second, "yarn", "list", "--json", "--depth=0") | ||
| duration := time.Since(start).Milliseconds() |
There was a problem hiding this comment.
scanYarnGlobal now executes via runInDir(..., "yarn", "list", ...) instead of the previous Windows cmd /c "cd ... && yarn ..." approach. The Windows unit test TestNodeScanner_ScanYarnGlobal_Windows still stubs the old cmd /c invocation, so the test suite will fail unless it’s updated to expect the new command form (or the mock’s RunInDir behavior is adjusted accordingly).
Adopt upstream's IDEType naming (intellij_idea, pycharm instead of intellij_idea_ultimate, pycharm_professional). Keep our Windows enhancements (glob paths, product-info.json, .eclipseproduct, RunInDir, JetBrains plugin detection). Incorporate upstream additions (Fleet, Xcode, Homebrew, Python scanning, UserAwareExecutor).
- resolveInstallDir: use directory mtime instead of lexicographic sort to pick the newest JetBrains installation (fixes 2024.9 vs 2024.10) - runInDir RunAsUser path: shell-quote name and args via platformShellQuote to prevent issues with spaces/metacharacters - Fix stale yarn global Windows test to match RunInDir call shape
When hardcoded WinPaths don't match (e.g., user installed to a custom path like D:\Tools\VSCode), fall back to querying the Windows registry Uninstall keys for InstallLocation. Zero performance impact for standard installs — registry is only queried when hardcoded paths fail. - Add registryInstallInfo struct + readRegistryInstallInfo (parses both DisplayVersion and InstallLocation from reg query output) - Refactor readRegistryVersion to delegate (backward-compatible) - Add RegistryName field to ideSpec for ambiguous app names - Refactor detectWindows into Phase 1 (hardcoded) + Phase 2 (registry) - 8 new tests covering custom paths, missing dirs, HKCU, spaces
The 'Reached data size limit' message was written via log.Progress() which is suppressed in quiet mode (enterprise default). This caused Step 21 of the integration tests to fail — the test looks for this message in the binary output but couldn't find it. Fix: Add Logger.Warn() that always prints regardless of quiet mode (like Error but with [warning] tag). Use Warn for size limit messages since they're important operational signals that should always be visible.
What does this PR do?
Type of change
Testing
./stepsecurity-dev-machine-guard --verbose./stepsecurity-dev-machine-guard --json | python3 -m json.toolmake lintmake testRelated Issues