[PPSC-697] feat: agent discovery#150
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
| Severity | Count |
|---|---|
| 🟠 HIGH | 5 |
| 🟡 MEDIUM | 5 |
Total: 10
View all 10 findings
🟠 HIGH (5)
CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') - Broken Access Control (CWE-22
Location: internal/agentdetect/platform_linux.go:29
Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')): The function JetBrainsPluginDirs builds a file path by taking the homeDir argument and appending a fixed sub‑directory (.local/share/JetBrains). It does this with filepath.Join and then passes the result to globJetBrainsPluginDirs. Because the code does not check or clean the homeDir value, an attacker who can control that argument could include “..” components (for example, ../../etc) to move the path outside the intended user home directory. The resulting path would be used by the glob function, which could cause the program to read files or directories that it should not access. In this snippet there is no special validation or sanitising step before the path is built, so the risky operation can be reached directly from the supplied homeDir. The function itself is not exposed as a network service or public API, so the issue would only be exploitable if some part of the program that receives external input eventually calls this function with that input. To protect against this, the code should verify that homeDir is a legitimate absolute path that stays inside the expected base directory, or reject any path components that try to traverse upward.
CWEs: CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') - Broken Access Control (CWE-22
Location: internal/agentdetect/platform_linux.go:25
Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')): The function VSCodeExtensionsDir builds a file system path by simply joining the homeDir argument with the fixed sub‑directories “.vscode/extensions”. It does not check whether the homeDir value is safe or whether it points outside the intended user directory. Because the function directly uses whatever string is passed in, an attacker who can influence the homeDir argument could cause the program to look for files in arbitrary locations, potentially accessing or modifying data it should not. No validation or sanitising steps are present before the path is created, so the risky operation is exposed to any uncontrolled input that reaches this function. This matches the description of a path‑traversal weakness (CWE‑22). The code itself does not show any external interface (like a web server) that would let a remote user provide homeDir, so the issue is not directly reachable from outside the program, but the flaw exists wherever the function is used. Adding checks to ensure homeDir is a legitimate directory (for example, confirming it is under a known base path) would mitigate the problem.
CWEs: CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
CWE-367: Time-of-check Time-of-use (TOCTOU) Race Condition - The `CheckMCP` function first makes sure that the directory `mcpDir` (the “.continue/mcpServers” folder) is inside the trusted home directory
Location: internal/agentdetect/detector.go:438
The CheckMCP function first makes sure that the directory mcpDir (the “.continue/mcpServers” folder) is inside the trusted home directory. It does this check with isUnderDir. After the check, the code reads the list of files in that folder and, for each file, calls HasArmisMCP on the full path. The problem is that the check only verifies the directory itself, not the individual files that are later processed. An attacker who can create or modify files in the “mcpServers” folder could add a file whose name includes path‑traversal components (for example ../../etc/passwd). When the loop reaches that file, HasArmisMCP will be called with the unvalidated path, allowing the program to read or act on a file outside the trusted home directory. This situation matches a Time‑of‑Check‑Time‑of‑Use (TOCTOU) race condition: the safety check is performed before the directory contents are examined, and the contents can change in the meantime. The code runs as part of a local detection routine; it is not exposed through a network service or public API. An attacker would need the ability to write files in the user’s home directory to exploit the issue. No additional validation or sanitisation of the file names is performed, so the risky operation can be reached directly from attacker‑controlled input.
CWEs: CWE-367: Time-of-check Time-of-use (TOCTOU) Race Condition
CWE-367: Time-of-check Time-of-use (TOCTOU) Race Condition - The function `dirExists` first checks that the given path is inside a trusted directory by calling `isUnderDir`
Location: internal/agentdetect/detector.go:48
The function dirExists first checks that the given path is inside a trusted directory by calling isUnderDir. After that check it calls os.Stat on the original path. Because the check and the use happen in two separate steps, an attacker who can change the filesystem (for example, replace a symlink) between those two calls could make the check succeed but cause os.Stat to look at a different location. This race condition lets the attacker bypass the directory restriction. The path that reaches dirExists comes from values like homeDir, which can be influenced by the user, and there is no additional validation before the os.Stat call. Therefore the risky operation can be triggered by user‑controlled input, and the code does not protect against the time‑of‑check‑time‑of‑use problem.
CWEs: CWE-367: Time-of-check Time-of-use (TOCTOU) Race Condition
CWE-770: Allocation of Resources Without Limits or Throttling - The function `readJSONFileAsMap` reads an entire file into memory using `os.ReadFile` and does not check how big the file is
Location: internal/install/editors.go:310
The function readJSONFileAsMap reads an entire file into memory using os.ReadFile and does not check how big the file is. The name of the file it reads comes from other parts of the program that build the path based on environment variables such as XDG_CONFIG_HOME or APPDATA. If an attacker can influence those environment variables (for example, by running the program with a custom environment), they can cause the program to open a very large file. Because there is no limit on the size, the program may allocate a huge amount of memory, which can lead to a crash or a denial‑of‑service condition. No part of the code limits the size or validates the file before reading it, so the risky operation is reachable from external input.
CWEs: CWE-770: Allocation of Resources Without Limits or Throttling
🟡 MEDIUM (5)
CWE-367: Time-of-check Time-of-use (TOCTOU) Race Condition - The `fileExists` function first checks that the given path is inside the allowed home directory by calling `isUnderDir`
Location: internal/agentdetect/detector.go:57
The fileExists function first checks that the given path is inside the allowed home directory by calling isUnderDir. Right after that check it calls os.Stat to look at the file. Between the check and the Stat call an attacker could change what the path points to (for example by replacing a file with a symbolic link). Because the function does not verify the path again after the Stat, it may report that a file exists (or does not exist) based on the attacker‑controlled location. This race condition can be triggered whenever the path argument comes from outside the program, such as a value supplied by a user or another part of the system. No extra validation or re‑checking is performed, so the risky operation can be abused to make the program trust the wrong file.
CWEs: CWE-367: Time-of-check Time-of-use (TOCTOU) Race Condition
CWE-770: Allocation of Resources Without Limits or Throttling - The function builds a file‑search pattern and then asks the operating system to find every directory that matches it
Location: internal/agentdetect/userprofile.go:55
The function builds a file‑search pattern and then asks the operating system to find every directory that matches it. The list of matches is stored in a slice and returned without any check on how many items were found. If the pattern matches a very large number of directories, the program could allocate a lot of memory just to hold the list, which can cause the program to run out of memory or become very slow. In this code the pattern is created from the baseDir argument, so the amount of memory used depends on what path is supplied. The function itself is not exposed directly to users over a network, but it could be called by other parts of the program that read configuration values or other internal data. Because the code does not limit the number of matches, an attacker who can influence the baseDir value (for example, by providing a specially crafted configuration) could cause the program to allocate excessive memory. No special checks or limits are present before the matches are stored, so the risky operation is the unbounded allocation of the slice that holds all the matching paths. Adding a limit on how many paths are collected, or validating the input directory, would prevent this problem.
CWEs: CWE-770: Allocation of Resources Without Limits or Throttling
CWE-770: Allocation of Resources Without Limits or Throttling - The function `enumerateUserDirs` reads every entry in a given directory and adds each one to a slice called `users`
Location: internal/agentdetect/userprofile.go:30
The function enumerateUserDirs reads every entry in a given directory and adds each one to a slice called users. It does this without checking how many entries there might be. If someone can choose a directory that contains a huge number of sub‑folders, the slice will keep growing and the program could use a lot of memory, possibly leading to a crash or slowdown. The code itself does not limit the number of items it stores, which matches the description of CWE‑770. Because the function does not receive any special protection or limit, the risky operation (the append on line 30) can consume as much memory as the directory provides. No special checks or safeguards are present to stop this from happening.
CWEs: CWE-770: Allocation of Resources Without Limits or Throttling
CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') - Broken Access Control (CWE-22
Location: internal/agentdetect/mcpconfig.go:23
Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')): The function tries to read a file whose location is given by the configPath argument. Before reading, it calls a helper called isUnderDir to check that the path stays inside a specific directory (resolvedHome). The code assumes this check is enough, but we don't see how isUnderDir works. If that helper does not correctly block tricks like “../” sequences, an attacker who can influence configPath could cause the program to read any file on the system that the process can access. This is the classic “path traversal” problem. Because the function directly opens the file with os.ReadFile, the risky operation (reading a file) will happen as soon as a malicious path reaches it. No additional validation or sanitizing steps are shown, so the vulnerable code can be reached. The function itself is not a public web endpoint, but it could be called by other parts of the program that receive data from elsewhere. That means the issue is not directly exposed to the internet, but it could still be exploited if an attacker can influence the configPath argument through any internal call chain. To fix the problem, make sure the path check correctly normalizes the input and rejects any attempt to escape the intended directory, or use a safe library function that enforces the restriction.
CWEs: CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
CWE-253: Incorrect Check of Function Return Value - The code tries to look up an editor by its ID with `install.EditorByID(id)`
Location: internal/cmd/install.go:191
The code tries to look up an editor by its ID with install.EditorByID(id). This function returns both the editor object and an error value. At line 191 the error is ignored (e, _ := install.EditorByID(id)), so if the lookup ever failed, the variable e could be nil. Later the code calls e.Register(...) and accesses e.Name, which would cause a crash if e were nil. Because the command‑line arguments are supplied by the user, this part of the program is reachable from a user‑facing interface. An attacker could try to trigger the situation by providing an editor name that the program does not recognize. However, the code already checks the editor’s existence earlier (lines 170‑174) and only adds known IDs to the list, so the nil‑pointer path is unlikely to be reached in practice. Even though the crash may be hard to provoke, the code still ignores the error return, which is exactly what CWE‑253 describes. Adding a proper check for the error (or removing the second call to EditorByID) would fix the issue.
CWEs: CWE-253: Incorrect Check of Function Return Value
Test Coverage Reporttotal: (statements) 77.2% Coverage by function |
There was a problem hiding this comment.
Pull request overview
Adds a new armis-cli agent-detection command to scan user profiles for installed AI coding agents and report whether Armis AppSec MCP is configured, and extends install to support additional editor targets.
Changes:
- Introduces
internal/agentdetectscanning framework with per-agent detectors, platform-specific user/profile discovery, and plain/JSON output formatters. - Adds the
agent-detectionCobra command wiring and output format flag validation. - Extends installable editor targets to include Roo Code and Junie, and documents OpenHands as manual-only.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/install/editors.go | Adds Roo Code/Junie as auto-configurable editors and their default config paths. |
| internal/cmd/install.go | Updates install help text and adds manual guidance for OpenHands. |
| internal/cmd/agent_detection.go | Adds new agent-detection CLI command with --format support. |
| internal/agentdetect/platform.go | Defines the Platform interface and UserHome model for cross-OS scanning. |
| internal/agentdetect/platform_darwin.go | macOS implementation for user enumeration and editor-specific paths. |
| internal/agentdetect/platform_linux.go | Linux implementation for user enumeration and editor-specific paths. |
| internal/agentdetect/platform_windows.go | Windows implementation for user enumeration and editor-specific paths + admin detection. |
| internal/agentdetect/userprofile.go | Shared helpers for enumerating home directories and JetBrains plugin paths. |
| internal/agentdetect/agent.go | Defines agent names and the detector registry. |
| internal/agentdetect/agentdetect.go | Implements the scanner and result models (grouped + flat). |
| internal/agentdetect/detector.go | Implements per-agent detection, MCP checks, and some version extraction. |
| internal/agentdetect/mcpconfig.go | Implements MCP presence checks for multiple config formats. |
| internal/agentdetect/format.go | Implements plain and json output writers. |
| internal/agentdetect/agentdetect_test.go | Unit tests for scanner aggregation and output formatters. |
| internal/agentdetect/detector_test.go | Unit tests for each detector + path traversal safety checks. |
| internal/agentdetect/mcpconfig_test.go | Unit tests for MCP config parsing helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if _, err := fmt.Fprintf(w, "user: %s\n", userResult.User); err != nil { | ||
| return err | ||
| } | ||
| agents := make([]string, 0, len(userResult.Agents)) | ||
| for _, agent := range userResult.Agents { | ||
| agents = append(agents, fmt.Sprintf("%s(MCP:%t)", agent.Name, agent.MCPInstalled)) | ||
| } | ||
| if _, err := fmt.Fprintln(w, strings.Join(agents, ", ")); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
The PR description says the plain output is "one line per user", but this formatter prints two lines per user (a "user:" header line plus a second line listing agents). Either adjust the plain formatter to emit a single line per user, or update the command/README text to match the actual output format so downstream parsing expectations don't break.
| if err != nil { | ||
| return false | ||
| } | ||
| return strings.HasPrefix(resolved, resolvedBase+string(filepath.Separator)) |
There was a problem hiding this comment.
isUnderDir relies on a case-sensitive strings.HasPrefix comparison. On Windows (case-insensitive paths), this can produce false negatives if resolvedBase and resolved differ only by drive letter or casing, which would cause agent detection to miss valid paths. Consider switching to a filepath.Rel-based check (rel not starting with ".." and not ".") and, on Windows, normalizing/strings.EqualFold-ing the volume/case before comparison.
| return strings.HasPrefix(resolved, resolvedBase+string(filepath.Separator)) | |
| baseVol := filepath.VolumeName(resolvedBase) | |
| resolvedVol := filepath.VolumeName(resolved) | |
| if baseVol != "" && resolvedVol != "" && strings.EqualFold(baseVol, resolvedVol) { | |
| resolved = baseVol + strings.TrimPrefix(resolved, resolvedVol) | |
| } | |
| rel, err := filepath.Rel(resolvedBase, resolved) | |
| if err != nil { | |
| return false | |
| } | |
| return rel != "." && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) |
| } | ||
|
|
||
| func (d *clineDetector) CheckMCP(resolvedHome, homeDir string, _ Platform) bool { | ||
| return HasArmisMCP(resolvedHome, filepath.Join(homeDir, ".cline", "mcp_settings.json")) |
There was a problem hiding this comment.
The clineDetector.CheckMCP() reads ~/.cline/mcp_settings.json, but armis-cli install cline writes to ~/Library/Application Support/Code/User/globalStorage/saoudrizwan.claude-dev/settings/cline_mcp_settings.json. After a user installs and then runs detection, Cline will always report MCP:false — the two features disagree on where Cline stores its config.
There was a problem hiding this comment.
There's quite a bit of repeat here, I suggest defining an AgentConfig struct with Name, ConfigDirs []string, MCPConfigPath string, ExtensionPrefix string fields. Implement a generic detector that reads from config. Keep custom detectors (Claude Code's enabledPlugins, Continue's directory scanning) as special cases.
There was a problem hiding this comment.
Good point, there's definitely repetition here. I looked into a config-driven approach but ~half the agents have enough quirks (Claude's enabledPlugins, Continue's directory scanning, Zed's context_servers, Copilot reading from VSCode's config dir, etc.) that the generic struct would need several escape hatches. I'd prefer to keep the explicit detectors for now and revisit if/when we add more agents. Happy to discuss if you feel strongly about it though.
| result := &ScanResult{} | ||
| for _, user := range users { | ||
| resolvedHome, err := resolvePath(user.HomeDir) | ||
| if err != nil { |
There was a problem hiding this comment.
When resolvePath(user.HomeDir) fails, the scan silently continues without any indication. Maybe add a Skipped []SkippedUser field to ScanResult or log warnings to stderr: fmt.Fprintf(os.Stderr, "warning: skipping user %s: %v\n", user.Username, err).?
There was a problem hiding this comment.
The plain formatter outputs raw fmt.Fprintf text while every other user-facing output in the project uses the lipgloss styling pipeline (documented in CLAUDE.md). The output looks disconnected from the rest of the CLI.
| return false | ||
| } | ||
| for _, entry := range entries { | ||
| if strings.Contains(strings.ToLower(entry.Name()), "armis") { |
There was a problem hiding this comment.
This seems to be filename-based only, while every other detector parses JSON content. A stale/empty armis-old.json or a armis-appsec.json.bak editor backup will falsely report "MCP installed = true".
am i missing something?
Related Issue
Type of Change
Problem
Armis Cloud has no visibility into which AI coding agents (Claude Code, Cursor, Copilot, Windsurf, etc.) are installed across developer workstations, nor whether those agents have the Armis AppSec MCP server configured. This data is needed to track agent adoption and MCP deployment coverage.
Solution
Add a new
armis-cli agent-detectioncommand that scans the local filesystem for 15 AI coding agents and reports whether each has Armis AppSec MCP installed.Agent detection (
internal/agentdetect/):Output formats:
--format plain(default): grouped by user, one line per user with agents and MCP status--format json: flat array ofDetectedAgentobjectsInstall command updates:
Testing
Automated Tests
Manual Testing
armis-cli agent-detection— verified detection of locally installed agents (Claude Code, Cursor) with correct MCP statussudo armis-cli agent-detection --format json— scan all users on the machine, JSON outputarmis-cli install roocode/armis-cli install junie— verified new editor targets workReviewer Notes
Checklist