Add ai_tools fleetd table#49243
Open
dantecatalfamo wants to merge 6 commits into
Open
Conversation
Vendor the karmine05/agentic-detector osquery extension (v0.3.0, commit 7c942d0) into orbit/pkg/table/ai_tools/ and register the unified `ai_tools` table cross-platform in OrbitDefaultTables(), so plain-osquery deployments get the table via the fleetd tables extension. The extension exposes a single `ai_tools` table with a `type` discriminator covering mcp_server, ide_plugins, agents, apps, sockets, agent_instruction, and browser_extension, plus risk_flags/sha256/detail columns. Imported as-is: internal/* collectors and the tables package copied verbatim with the import prefix rewritten and the package renamed to ai_tools. Added an apps stub for the non-darwin/linux/windows fallback build and exported Columns()/Generate() wrappers for registration. Adjusted the copied source to satisfy Fleet's linters (set types, modernize idioms, defensive nil guards); all changes are behavior-preserving. Deps (gopsutil/v4, howett.net/plist, gopkg.in/yaml.v3, osquery-go, golang.org/x/sys) already exist in go.mod at Fleet's pinned versions; only yaml.v3 changed from indirect to direct.
The ai_tools table runs in-process in the root/SYSTEM orbit daemon and walks, reads, and hashes files across every user's home directory. A prior audit of the vendored v0.3.0 source found five ways an unprivileged local user could abuse that. Fix each while preserving behavior for legitimate inputs: - FIFO / special-file DoS: fsutil now opens files via OpenRegular, which Lstat-checks for a regular file and opens O_NONBLOCK, so a named pipe or device planted at a scanned path can no longer block the daemon. SHA256, Exists, and the new ReadFileBounded all go through it, and every raw os.ReadFile / os.Open in the collectors now routes through the safe helpers. - Symlink hash/content disclosure: the same Lstat + IsRegular check refuses symlinks, so a low-priv user can no longer point a scanned path at a root-only file to leak its hash or content signals. Agent binaries under trusted system bin dirs (Homebrew et al.) are still resolved and hashed. - Path traversal via attacker-controlled fields: the Chromium Preferences "path", Gecko addon id, and macOS CFBundleExecutable are now contained to the owning home / rejected when they contain separators or "..". - Outbound DNS egress: netsock no longer resolves hostnames taken from untrusted MCP config files; egress is attributed by owning process, which is DNS-free. - No panic recovery: the exported Generate wrapper now recovers from any collector panic and returns it as a query error, so one malformed file cannot crash the daemon's custom-table surface. Adds regression tests for symlink/FIFO refusal and addon-id traversal. Not addressed (informational): homes.All trusts the home directory name for the uid/username attribution columns; enumerating from the OS user database would be a follow-up.
dantecatalfamo
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #49243 +/- ##
==========================================
- Coverage 67.95% 67.91% -0.04%
==========================================
Files 3766 3797 +31
Lines 238601 242088 +3487
Branches 12465 12465
==========================================
+ Hits 162132 164418 +2286
- Misses 61816 62708 +892
- Partials 14653 14962 +309
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the new ai_tools osquery table to Orbit’s default (fleetd tables) extension so it’s available to hosts running plain osquery with the fleetd tables extension (e.g. via ufa). The implementation vendors the upstream “agentic-detector” collectors into orbit/pkg/table/ai_tools/, wires the table into OrbitDefaultTables(), and includes a suite of unit tests plus an opt-in live-host smoke test.
Changes:
- Register
ai_toolsinOrbitDefaultTables()and expose exportedColumns()/Generate()wrappers for extension registration. - Add a unified
ai_toolstable implementation with collector pushdown by discriminator and type-specificdetailJSON. - Add/retain unit tests across collectors and promote
gopkg.in/yaml.v3to a direct dependency.
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| orbit/pkg/table/extension.go | Registers the ai_tools plugin in Orbit’s default table set. |
| orbit/pkg/table/ai_tools/tables.go | Defines unified schema, constraint pushdown, and row mapping for ai_tools. |
| orbit/pkg/table/ai_tools/ai_tools.go | Exports Columns()/Generate() wrappers and adds panic recovery around generation. |
| orbit/pkg/table/ai_tools/smoke_test.go | Adds opt-in live-host smoke test for the unified table + pushdown. |
| orbit/pkg/table/ai_tools/browserext_row_test.go | Unit tests for browser extension row mapping and type registration. |
| orbit/pkg/table/ai_tools/internal/proc/proc.go | Process/connection snapshot used by multiple collectors to avoid repeated enumeration. |
| orbit/pkg/table/ai_tools/internal/paths/paths.go | Resolves per-OS app-data roots from a home directory. |
| orbit/pkg/table/ai_tools/internal/netsock/netsock.go | Classifies listening/egress sockets into AI/MCP-related categories. |
| orbit/pkg/table/ai_tools/internal/netsock/netsock_test.go | Tests socket classification behavior (including loopback IPC exclusion). |
| orbit/pkg/table/ai_tools/internal/mcp/mcp.go | Discovers MCP servers from client config files across platforms and scopes. |
| orbit/pkg/table/ai_tools/internal/mcp/correlate.go | Correlates declared MCP servers with running processes; adds process-only discoveries. |
| orbit/pkg/table/ai_tools/internal/mcp/risk.go | Computes MCP posture/risk flags (fetch-and-run, secrets, cleartext, perms, capabilities). |
| orbit/pkg/table/ai_tools/internal/mcp/mcp_test.go | Tests MCP config scanning and process correlation. |
| orbit/pkg/table/ai_tools/internal/mcp/risk_test.go | Tests MCP risk flag derivation and helper behaviors (pinning, secret env detection). |
| orbit/pkg/table/ai_tools/internal/instructions/instructions.go | Discovers agent instruction files and flags injection/hidden-unicode/world-writable risks. |
| orbit/pkg/table/ai_tools/internal/instructions/instructions_test.go | Tests instruction discovery, injection marker detection, hidden unicode, and perms flags. |
| orbit/pkg/table/ai_tools/internal/ide/ide.go | Entry point for IDE/plugin discovery across editor families. |
| orbit/pkg/table/ai_tools/internal/ide/vscode.go | VS Code family extension scanning and AI classification (AI-only emission). |
| orbit/pkg/table/ai_tools/internal/ide/jetbrains.go | JetBrains plugin scanning via plugin.xml discovery in dirs/jars. |
| orbit/pkg/table/ai_tools/internal/ide/others.go | Scanners for Zed/Sublime/Vim/Emacs and minimal manifest parsing helpers. |
| orbit/pkg/table/ai_tools/internal/ide/ide_test.go | Tests VS Code scanning, obsolete filtering, ELPA parsing, and JetBrains editor label derivation. |
| orbit/pkg/table/ai_tools/internal/homes/homes.go | Enumerates real user home directories across platforms. |
| orbit/pkg/table/ai_tools/internal/fsutil/fsutil.go | Safe filesystem helpers (bounded reads/hashes, perms, bounded directory walks). |
| orbit/pkg/table/ai_tools/internal/fsutil/fsutil_unix_test.go | Tests symlink refusal and FIFO non-blocking behavior on non-Windows. |
| orbit/pkg/table/ai_tools/internal/fsutil/fsutil_test.go | Tests hashing, perms, bounded walking, and Exists behavior. |
| orbit/pkg/table/ai_tools/internal/classify/kb.json | Embedded knowledge base for AI identifiers/regex/ports/capabilities. |
| orbit/pkg/table/ai_tools/internal/classify/classify.go | Loads embedded KB and exposes classification helpers (plugins/cmdline/ports/capabilities/exts). |
| orbit/pkg/table/ai_tools/internal/classify/classify_test.go | Tests classification behaviors for VS Code plugins, cmdlines, ports, capabilities, browser extensions. |
| orbit/pkg/table/ai_tools/internal/browserext/browserext.go | Scans Chromium/Gecko browser profiles for AI extensions across users. |
| orbit/pkg/table/ai_tools/internal/browserext/chromium.go | Chromium extension inventory (disk + prefs union), i18n name resolution, path containment. |
| orbit/pkg/table/ai_tools/internal/browserext/gecko.go | Gecko extension inventory from extensions.json + XPI manifest fallback; traversal rejection. |
| orbit/pkg/table/ai_tools/internal/browserext/risk.go | Browser extension risk flags (broad hosts, sideloading/unverified). |
| orbit/pkg/table/ai_tools/internal/browserext/browserext_test.go | Tests Chromium/Gecko extension collection, risk flags, traversal rejection, profiles discovery, Scan integration. |
| orbit/pkg/table/ai_tools/internal/apps/apps.go | Cross-platform AI desktop app detection orchestration + liveness + hashing. |
| orbit/pkg/table/ai_tools/internal/apps/apps_darwin.go | macOS app bundle scanning + Info.plist parsing with executable path hardening. |
| orbit/pkg/table/ai_tools/internal/apps/apps_linux.go | Linux .desktop scanning and service-binary fallback detection. |
| orbit/pkg/table/ai_tools/internal/apps/apps_windows.go | Windows uninstall-registry scanning for known AI apps. |
| orbit/pkg/table/ai_tools/internal/apps/apps_stub.go | Stub for unsupported platforms to keep builds working. |
| orbit/pkg/table/ai_tools/internal/apps/apps_test.go | Tests known-app matching logic. |
| orbit/pkg/table/ai_tools/internal/agents/agents.go | Detects known AI agent CLIs from disk (no exec), versions from manifests, runtime posture from process snapshot. |
| orbit/pkg/table/ai_tools/internal/agents/posture.go | Enriches autonomy posture/risk flags (Claude settings + runtime flags). |
| orbit/pkg/table/ai_tools/internal/agents/agents_test.go | Tests agent detection and running-process matching. |
| orbit/pkg/table/ai_tools/internal/agents/posture_test.go | Tests permission mode parsing and posture risk flagging. |
| orbit/changes/47619-ai-tools-table | Adds Orbit change log entry for the new ai_tools table. |
| go.mod | Promotes gopkg.in/yaml.v3 to a direct dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+42
to
+46
| if !fi.Mode().IsRegular() { | ||
| return nil, os.ErrInvalid | ||
| } | ||
| return os.OpenFile(path, os.O_RDONLY|syscall.O_NONBLOCK, 0) // #nosec G304 -- path discovered by a curated collector; opened non-following, non-blocking, regular-only | ||
| } |
Comment on lines
+1
to
+5
| // Package tables exposes a single, unified osquery table — ai_tools — | ||
| // covering every AI-tool type (MCP servers, IDE plugins, AI agent | ||
| // CLIs, AI desktop apps, live AI/MCP sockets, agent instruction files, and browser extensions) | ||
| // through one schema with a `type` discriminator, security `risk_flags` and | ||
| // `sha256` columns, and a JSON `detail` column for type-specific fields. |
Comment on lines
+88
to
+92
| func generate(ctx context.Context, qc table.QueryContext) ([]map[string]string, error) { | ||
| types := requestedTypes(qc) | ||
| has := func(t string) bool { _, ok := types[t]; return ok } | ||
|
|
||
| hs := homes.All() |
Comment on lines
+46
to
+50
| // columns is the unified schema. Common fields are first-class; everything | ||
| // type-specific lives in `detail` (compact JSON, empty fields omitted). | ||
| var columns = []string{ | ||
| "type", // mcp_server | ide_plugins | agents | apps | sockets | agent_instruction | browser_extension | ||
| "name", // server/plugin/agent/app/process/instruction-file name |
Comment on lines
+37
to
+49
| // Collect classifies the snapshot's connections. remoteMCPHosts maps a hostname | ||
| // parsed from a remote MCP config to a label; those hosts are resolved to IPs so | ||
| // established connections can be attributed even without per-connection reverse | ||
| // DNS. | ||
| func Collect(ctx context.Context, snap *proc.Snapshot, remoteMCPHosts map[string]string) []Socket { | ||
| if snap == nil { | ||
| return nil | ||
| } | ||
| hosts := buildHostSet(remoteMCPHosts) | ||
| var out []Socket | ||
|
|
||
| for _, c := range snap.Conns { | ||
| p := snap.Procs[c.PID] |
| // unconstrained (all types), then with a type='ide_plugins' constraint to prove | ||
| // pushdown returns only that type. Opt-in (AED_SMOKE=1) — reads live state. | ||
| // | ||
| // AED_SMOKE=1 go test -run TestSmokeLiveHost -v ./tables/ |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
🚧 Files skipped from review as they are similar to previous changes (10)
WalkthroughAdds a unified Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
orbit/pkg/table/ai_tools/internal/instructions/instructions.go (1)
189-191: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSingle
Readcall may silently truncate the scanned content.
f.Read(buf)isn't guaranteed to fillbufup tomaxReadBytesor EOF in one call; a short read (more plausible on networked home directories) would causeinjectionMarkers/hidden-unicode detection to silently miss content near/after the truncation point.♻️ Proposed fix
- buf := make([]byte, maxReadBytes) - n, _ := f.Read(buf) - content := string(buf[:n]) + buf := make([]byte, maxReadBytes) + n, _ := io.ReadFull(f, buf) // expected io.ErrUnexpectedEOF for files < maxReadBytes; n is still valid + content := string(buf[:n])(requires adding
"io"to imports)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orbit/pkg/table/ai_tools/internal/instructions/instructions.go` around lines 189 - 191, Update the file-reading logic in the surrounding instruction-loading function to use io.ReadAllLimit (or an equivalent io-based loop) instead of relying on the single f.Read call. Preserve the maxReadBytes limit, ensure short reads continue until EOF or the limit is reached, and retain the existing string conversion and downstream detection behavior.orbit/pkg/table/ai_tools/internal/proc/proc.go (1)
45-76: 🩺 Stability & Availability | 🔵 TrivialConsider a hard deadline around process/connection enumeration.
ctx.Err()is only checked between iterations (Line 50); a single stuck syscall inside gopsutil (e.g.UsernameWithContexthitting a slow NSS backend, orExeWithContexton a hung network mount) won't be interrupted by context cancellation. Since this runs in-process in the root/SYSTEM orbit daemon and the table already added panic recovery for similar "one bad artifact shouldn't crash everything" reasons, consider givingTake(or its caller intables.go) an explicitcontext.WithTimeoutbudget so a single hung syscall degrades gracefully instead of stalling the whole query.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orbit/pkg/table/ai_tools/internal/proc/proc.go` around lines 45 - 76, Update Take to enforce an explicit context timeout covering both process and connection enumeration, using a bounded budget appropriate for the orbit daemon. Derive the timeout context at the start of Take, pass it to all gopsutil enumeration and per-process calls, and ensure cancellation causes enumeration to stop and the snapshot to return rather than waiting indefinitely on a single syscall.orbit/pkg/table/ai_tools/internal/classify/classify.go (1)
41-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPanic on invalid regex patterns for consistency with JSON parsing.
The
init()function panics onjson.Unmarshalfailure (line 39) but silently skips regex compilation errors. BothkbBytesanddata.NameRegexare embedded, in-repo data — an invalid regex is equally a build-time bug. Silently skipping would cause missing classification patterns to go unnoticed.♻️ Proposed fix
for _, p := range data.NameRegex { - if re, err := regexp.Compile(p); err == nil { - nameRes = append(nameRes, re) - } + re, err := regexp.Compile(p) + if err != nil { + panic("classify: invalid regex in kb.json " + strconv.Quote(p) + ": " + err.Error()) + } + nameRes = append(nameRes, re) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orbit/pkg/table/ai_tools/internal/classify/classify.go` around lines 41 - 45, Update the regex compilation loop in init to panic when regexp.Compile fails, matching the existing json.Unmarshal failure behavior for embedded data. Do not silently skip invalid patterns; preserve appending successfully compiled expressions to nameRes.orbit/pkg/table/ai_tools/internal/browserext/chromium.go (1)
93-112: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLexical version-dir comparison can pick a stale extension version.
e.Name() > bestcompares version directory names as plain strings. For multi-digit version components (e.g."9.0.0.0"vs"10.0.0.0"), lexical ordering picks"9.0.0.0"as "greater," so during an update window where both directories briefly coexist, the collector could report metadata/hash for the older, not the latest, version.♻️ Proposed fix: compare version segments numerically
- best := "" - for _, e := range entries { - if e.IsDir() && e.Name() > best { - best = e.Name() - } - } + best := "" + for _, e := range entries { + if e.IsDir() && (best == "" || compareVersions(e.Name(), best) > 0) { + best = e.Name() + } + }Where
compareVersionssplits on.and_and compares each numeric segment, falling back to string comparison on parse failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orbit/pkg/table/ai_tools/internal/browserext/chromium.go` around lines 93 - 112, Update latestVersionManifest to select the greatest version directory using compareVersions rather than the lexical e.Name() > best check. Preserve the existing directory filtering, empty-result handling, manifest existence validation, and return values.orbit/pkg/table/ai_tools/internal/mcp/risk.go (1)
14-20: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
fetchRunners["dlx"]likely never matches;pnpxis missing.
pnpm dlx <pkg>is invoked as base commandpnpmwithdlxas the first argument, not as a standalonedlxexecutable, so the"dlx"key in this map is effectively dead for its intended purpose. Meanwhilepnpx(the pnpm-dlx alias, a real standalone command) isn't in the map at all, so MCP servers launched viapnpm dlx ...orpnpx ...won't be flaggedremote_fetch_exec/unpinned_dependency.🔒 Proposed fix
var fetchRunners = map[string]struct{}{ "npx": {}, "npx.cmd": {}, "bunx": {}, "bunx.cmd": {}, "pnpx": {}, "pnpx.cmd": {}, "uvx": {}, "uvx.cmd": {}, - "dlx": {}, }And in
fetchSpec, special-casepnpm/pnpm.cmdwithargs[0] == "dlx"as an additional fetch-runner match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orbit/pkg/table/ai_tools/internal/mcp/risk.go` around lines 14 - 20, Update fetchRunners to include the standalone pnpx and pnpx.cmd commands, and remove the ineffective dlx entry. In fetchSpec, additionally classify pnpm and pnpm.cmd as fetch runners when args[0] is dlx, while preserving existing runner matching behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@orbit/pkg/table/ai_tools/internal/apps/apps.go`:
- Around line 42-58: Update the process matching used by markRunning so short
processNames tokens such as those in the Jan and Dia entries cannot match as
arbitrary substrings; require a process-name boundary or otherwise reuse the
padded, collision-resistant matching semantics already represented by match.
Preserve valid detection of the intended applications while preventing unrelated
process names from marking rows as running.
In `@orbit/pkg/table/ai_tools/internal/browserext/gecko.go`:
- Around line 133-173: The geckoProfiles path construction currently permits
relative Path values containing .. to escape root. Update the flush closure in
geckoProfiles to normalize each resolved profile path and retain it only when it
remains contained within the expected Gecko root, while continuing to allow
valid absolute profile paths and relative paths inside root; exclude escaped
profiles before collectGeckoProfile scans them.
In `@orbit/pkg/table/ai_tools/internal/fsutil/fsutil.go`:
- Around line 30-46: The OpenRegular function remains vulnerable to symlink
replacement between Lstat and OpenFile. Add O_NOFOLLOW to the open flags, using
the project’s platform-specific handling where required, while preserving the
existing read-only, nonblocking, regular-file checks.
In `@orbit/pkg/table/ai_tools/internal/ide/jetbrains.go`:
- Around line 135-156: The readPluginXMLFromJar function must validate the jar
path with fsutil.OpenRegular before opening it as a ZIP. Reuse the same
regular-file guard and error handling used by ReadFileBounded, then pass the
validated file handle or path into the archive-opening flow so symlinks and
special files cannot be followed or block the scan.
In `@orbit/pkg/table/ai_tools/internal/instructions/instructions.go`:
- Line 38: Update the RiskFlags field comment to replace world_readable with
world_writable, matching the flag emitted by the implementation and verified by
TestWorldWritableFlag. Keep the other documented flags unchanged.
- Around line 125-133: Update build so the Size lookup uses lstat semantics
rather than os.Stat, preventing symlinks from exposing target metadata while
preserving the existing behavior for regular files. Keep the SHA256 and other
Instruction fields unchanged, and use the existing filesystem utility or
equivalent non-following metadata call.
In `@orbit/pkg/table/ai_tools/tables.go`:
- Around line 210-238: Update mcpRow so the enabled field uses a descriptive
label for the meaningful disabled state instead of passing itoa(s.Enabled)
directly to compactJSON, which drops "0"; follow the existing
fromWebstoreStr/signedStateStr handling for analogous zero-valued states while
preserving the enabled representation for other states.
---
Nitpick comments:
In `@orbit/pkg/table/ai_tools/internal/browserext/chromium.go`:
- Around line 93-112: Update latestVersionManifest to select the greatest
version directory using compareVersions rather than the lexical e.Name() > best
check. Preserve the existing directory filtering, empty-result handling,
manifest existence validation, and return values.
In `@orbit/pkg/table/ai_tools/internal/classify/classify.go`:
- Around line 41-45: Update the regex compilation loop in init to panic when
regexp.Compile fails, matching the existing json.Unmarshal failure behavior for
embedded data. Do not silently skip invalid patterns; preserve appending
successfully compiled expressions to nameRes.
In `@orbit/pkg/table/ai_tools/internal/instructions/instructions.go`:
- Around line 189-191: Update the file-reading logic in the surrounding
instruction-loading function to use io.ReadAllLimit (or an equivalent io-based
loop) instead of relying on the single f.Read call. Preserve the maxReadBytes
limit, ensure short reads continue until EOF or the limit is reached, and retain
the existing string conversion and downstream detection behavior.
In `@orbit/pkg/table/ai_tools/internal/mcp/risk.go`:
- Around line 14-20: Update fetchRunners to include the standalone pnpx and
pnpx.cmd commands, and remove the ineffective dlx entry. In fetchSpec,
additionally classify pnpm and pnpm.cmd as fetch runners when args[0] is dlx,
while preserving existing runner matching behavior.
In `@orbit/pkg/table/ai_tools/internal/proc/proc.go`:
- Around line 45-76: Update Take to enforce an explicit context timeout covering
both process and connection enumeration, using a bounded budget appropriate for
the orbit daemon. Derive the timeout context at the start of Take, pass it to
all gopsutil enumeration and per-process calls, and ensure cancellation causes
enumeration to stop and the snapshot to return rather than waiting indefinitely
on a single syscall.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0f94b3a5-5e86-4ff3-8aeb-b014aa753b38
📒 Files selected for processing (45)
go.modorbit/changes/47619-ai-tools-tableorbit/pkg/table/ai_tools/ai_tools.goorbit/pkg/table/ai_tools/browserext_row_test.goorbit/pkg/table/ai_tools/internal/agents/agents.goorbit/pkg/table/ai_tools/internal/agents/agents_test.goorbit/pkg/table/ai_tools/internal/agents/posture.goorbit/pkg/table/ai_tools/internal/agents/posture_test.goorbit/pkg/table/ai_tools/internal/apps/apps.goorbit/pkg/table/ai_tools/internal/apps/apps_darwin.goorbit/pkg/table/ai_tools/internal/apps/apps_linux.goorbit/pkg/table/ai_tools/internal/apps/apps_stub.goorbit/pkg/table/ai_tools/internal/apps/apps_test.goorbit/pkg/table/ai_tools/internal/apps/apps_windows.goorbit/pkg/table/ai_tools/internal/browserext/browserext.goorbit/pkg/table/ai_tools/internal/browserext/browserext_test.goorbit/pkg/table/ai_tools/internal/browserext/chromium.goorbit/pkg/table/ai_tools/internal/browserext/gecko.goorbit/pkg/table/ai_tools/internal/browserext/risk.goorbit/pkg/table/ai_tools/internal/classify/classify.goorbit/pkg/table/ai_tools/internal/classify/classify_test.goorbit/pkg/table/ai_tools/internal/classify/kb.jsonorbit/pkg/table/ai_tools/internal/fsutil/fsutil.goorbit/pkg/table/ai_tools/internal/fsutil/fsutil_test.goorbit/pkg/table/ai_tools/internal/fsutil/fsutil_unix_test.goorbit/pkg/table/ai_tools/internal/homes/homes.goorbit/pkg/table/ai_tools/internal/ide/ide.goorbit/pkg/table/ai_tools/internal/ide/ide_test.goorbit/pkg/table/ai_tools/internal/ide/jetbrains.goorbit/pkg/table/ai_tools/internal/ide/others.goorbit/pkg/table/ai_tools/internal/ide/vscode.goorbit/pkg/table/ai_tools/internal/instructions/instructions.goorbit/pkg/table/ai_tools/internal/instructions/instructions_test.goorbit/pkg/table/ai_tools/internal/mcp/correlate.goorbit/pkg/table/ai_tools/internal/mcp/mcp.goorbit/pkg/table/ai_tools/internal/mcp/mcp_test.goorbit/pkg/table/ai_tools/internal/mcp/risk.goorbit/pkg/table/ai_tools/internal/mcp/risk_test.goorbit/pkg/table/ai_tools/internal/netsock/netsock.goorbit/pkg/table/ai_tools/internal/netsock/netsock_test.goorbit/pkg/table/ai_tools/internal/paths/paths.goorbit/pkg/table/ai_tools/internal/proc/proc.goorbit/pkg/table/ai_tools/smoke_test.goorbit/pkg/table/ai_tools/tables.goorbit/pkg/table/extension.go
Follow-up to the collector hardening, from an independent adversarial review: - fsutil.OpenRegular: close the stat->open TOCTOU race. The pre-open Lstat alone did not stop a symlink swapped in after the check (the open would still follow it). Add a post-open fstat that re-checks IsRegular and, via os.SameFile, confirms the opened file is the same inode Lstat saw; refuse otherwise. Document the ReadFileBounded truncation contract. - netsock: remove the now-dead remote-MCP host plumbing. With DNS resolution gone, the host set was keyed by hostname but looked up by IP, so it could never match. Drop buildHostSet, the hostInfo type, Collect's ctx and remoteMCPHosts params, and the dead attribution branch; fix the stale doc comment. tables.go no longer builds a remote-host map and only scans MCP configs for the mcp_server type. - Add regression tests: Chromium out-of-home Preferences path is recovered but not hashed, and a macOS CFBundleExecutable with path traversal yields no execPath. - Note the symlink-not-followed detection limitation in the changes file.
dantecatalfamo
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
orbit/pkg/table/ai_tools/tables.go (1)
143-146: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHonor query cancellation before scanning applications.
Unlike the other collectors, this branch invokes
apps.Scanwithout checkingctx.Err(). A canceled query can therefore still start the potentially expensive all-home application scan.Suggested fix
if has("apps") { + if err := ctx.Err(); err != nil { + return nil, err + } for _, a := range apps.Scan(hs, snap) {If the scan itself is long-running, propagate the context into
apps.Scanas well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orbit/pkg/table/ai_tools/tables.go` around lines 143 - 146, In the applications collector branch guarded by has("apps"), check ctx.Err() before starting apps.Scan and return the cancellation error instead of scanning when the query is already canceled. If supported by the API, also pass the context into apps.Scan so cancellation can stop an in-progress scan.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@orbit/pkg/table/ai_tools/internal/netsock/netsock.go`:
- Around line 61-65: Update the connection-state condition in the socket
classification switch to classify only connections whose status is explicitly
ESTABLISHED, or whose status is explicitly empty/unknown; remove the
RemotePort/RemoteIP fallback. Preserve the existing classifyEstablished(&sock)
behavior for accepted connections and continue ignoring transient states such as
TIME_WAIT and CLOSE_WAIT.
---
Outside diff comments:
In `@orbit/pkg/table/ai_tools/tables.go`:
- Around line 143-146: In the applications collector branch guarded by
has("apps"), check ctx.Err() before starting apps.Scan and return the
cancellation error instead of scanning when the query is already canceled. If
supported by the API, also pass the context into apps.Scan so cancellation can
stop an in-progress scan.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e0463137-dc06-4914-a7b2-0df566acd389
📒 Files selected for processing (7)
orbit/changes/47619-ai-tools-tableorbit/pkg/table/ai_tools/internal/apps/apps_darwin_test.goorbit/pkg/table/ai_tools/internal/browserext/browserext_test.goorbit/pkg/table/ai_tools/internal/fsutil/fsutil.goorbit/pkg/table/ai_tools/internal/netsock/netsock.goorbit/pkg/table/ai_tools/internal/netsock/netsock_test.goorbit/pkg/table/ai_tools/tables.go
🚧 Files skipped from review as they are similar to previous changes (4)
- orbit/changes/47619-ai-tools-table
- orbit/pkg/table/ai_tools/internal/netsock/netsock_test.go
- orbit/pkg/table/ai_tools/internal/browserext/browserext_test.go
- orbit/pkg/table/ai_tools/internal/fsutil/fsutil.go
- homes.All now derives each home's uid/username from the directory's actual owner (stat) instead of its name, so a low-privilege user who creates a directory named after another account (e.g. /Users/root) can no longer forge the uid/username attribution columns. Falls back to name-based lookup where the OS does not expose a numeric owner uid (Windows). Adds a regression test. - Added a README recording the vendoring provenance (upstream repo, tag, commit, the modifications made) and flagging that the upstream repository had no LICENSE at the vendored commit, which must be resolved before shipping.
dantecatalfamo
had a problem deploying
to
Docker Hub
GitHub Actions
Error
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@orbit/pkg/table/ai_tools/internal/homes/owner_unix_test.go`:
- Around line 33-38: Update the username assertion in the owner test around
owner so it compares username directly with cur.Username, replacing the
special-case root check. Preserve the existing UID assertion and ensure the test
validates the exact resolved UID-to-username attribution.
In `@orbit/pkg/table/ai_tools/internal/homes/owner_unix.go`:
- Line 1: Narrow the build constraint on owner_unix.go from !windows to explicit
GOOS tags for each supported Unix platform where syscall.Stat_t is available, or
provide a separate fallback implementation for unsupported targets. Keep the
existing Unix implementation unchanged for supported platforms.
In `@orbit/pkg/table/ai_tools/internal/homes/owner_windows.go`:
- Around line 7-10: Update the Windows statOwnerUID fallback path so owner()
does not use filepath.Base(dir) for attribution when ownership lookup fails.
Return and propagate an explicit unknown or unowned result on Windows,
preserving name-based attribution only for trusted directories if that existing
mechanism is available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dfab3353-548b-4e27-b722-a4ecb637b0e4
⛔ Files ignored due to path filters (1)
orbit/pkg/table/ai_tools/README.mdis excluded by!**/*.md
📒 Files selected for processing (4)
orbit/pkg/table/ai_tools/internal/homes/homes.goorbit/pkg/table/ai_tools/internal/homes/owner_unix.goorbit/pkg/table/ai_tools/internal/homes/owner_unix_test.goorbit/pkg/table/ai_tools/internal/homes/owner_windows.go
🚧 Files skipped from review as they are similar to previous changes (1)
- orbit/pkg/table/ai_tools/internal/homes/homes.go
Comment on lines
+33
to
+38
| uid, username := owner(dir, fi) | ||
| if uid != cur.Uid { | ||
| t.Errorf("uid = %q, want the real owner %q (must be read from stat, not the name)", uid, cur.Uid) | ||
| } | ||
| if username == "root" { | ||
| t.Errorf("username = %q, forged from the directory name; want the real owner %q", username, cur.Username) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the resolved username exactly.
Checking only username != "root" would let an incorrect mapping such as "nobody" pass. Assert username == cur.Username to verify the complete UID-to-username attribution.
Proposed test fix
- if username == "root" {
- t.Errorf("username = %q, forged from the directory name; want the real owner %q", username, cur.Username)
+ if username != cur.Username {
+ t.Errorf("username = %q, want the real owner %q", username, cur.Username)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| uid, username := owner(dir, fi) | |
| if uid != cur.Uid { | |
| t.Errorf("uid = %q, want the real owner %q (must be read from stat, not the name)", uid, cur.Uid) | |
| } | |
| if username == "root" { | |
| t.Errorf("username = %q, forged from the directory name; want the real owner %q", username, cur.Username) | |
| uid, username := owner(dir, fi) | |
| if uid != cur.Uid { | |
| t.Errorf("uid = %q, want the real owner %q (must be read from stat, not the name)", uid, cur.Uid) | |
| } | |
| if username != cur.Username { | |
| t.Errorf("username = %q, want the real owner %q", username, cur.Username) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@orbit/pkg/table/ai_tools/internal/homes/owner_unix_test.go` around lines 33 -
38, Update the username assertion in the owner test around owner so it compares
username directly with cur.Username, replacing the special-case root check.
Preserve the existing UID assertion and ensure the test validates the exact
resolved UID-to-username attribution.
| @@ -0,0 +1,20 @@ | |||
| //go:build !windows | |||
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for target in "linux amd64" "darwin amd64" "windows amd64" "plan9 amd64"; do
read -r goos goarch <<< "$target"
GOOS="$goos" GOARCH="$goarch" go test -c \
-o "/tmp/homes-${goos}-${goarch}.test" \
./orbit/pkg/table/ai_tools/internal/homes
doneRepository: fleetdm/fleet
Length of output: 472
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files orbit/pkg/table/ai_tools/internal/homes
printf '\n== owner_unix.go ==\n'
cat -n orbit/pkg/table/ai_tools/internal/homes/owner_unix.go
printf '\n== other homes files with build tags ==\n'
for f in $(git ls-files orbit/pkg/table/ai_tools/internal/homes); do
echo "--- $f"
sed -n '1,20p' "$f" | cat -n
doneRepository: fleetdm/fleet
Length of output: 3571
Narrow the build tag to supported Unix targets.
!windows includes every non-Windows GOOS, but syscall.Stat_t isn’t available on all of them, so this package can fail to build on unsupported platforms. Use explicit GOOS tags for the platforms you support, or add a fallback implementation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@orbit/pkg/table/ai_tools/internal/homes/owner_unix.go` at line 1, Narrow the
build constraint on owner_unix.go from !windows to explicit GOOS tags for each
supported Unix platform where syscall.Stat_t is available, or provide a separate
fallback implementation for unsupported targets. Keep the existing Unix
implementation unchanged for supported platforms.
Comment on lines
+7
to
+10
| // statOwnerUID has no portable implementation on Windows (ownership is | ||
| // expressed via security descriptors / SIDs, not a numeric uid), so callers | ||
| // fall back to name-based attribution there. | ||
| func statOwnerUID(_ os.FileInfo) (string, bool) { return "", false } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the target file and all references to statOwnerUID
git ls-files 'orbit/pkg/table/ai_tools/internal/homes/*' 'orbit/pkg/table/ai_tools/**' | sed -n '1,200p'
printf '\n--- references ---\n'
rg -n "statOwnerUID|owner attribution|name-based|security descriptor|SID|owner" orbit/pkg/table/ai_tools -S
printf '\n--- target file ---\n'
cat -n orbit/pkg/table/ai_tools/internal/homes/owner_windows.goRepository: fleetdm/fleet
Length of output: 6119
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- homes.go ---'
sed -n '60,100p' orbit/pkg/table/ai_tools/internal/homes/homes.go
printf '\n%s\n' '--- README.md excerpt ---'
sed -n '20,45p' orbit/pkg/table/ai_tools/README.md
printf '\n%s\n' '--- owner_unix_test.go excerpt ---'
sed -n '1,60p' orbit/pkg/table/ai_tools/internal/homes/owner_unix_test.goRepository: fleetdm/fleet
Length of output: 3723
Don't fall back to directory-name attribution on Windows. owner() uses filepath.Base(dir) when statOwnerUID fails, so a folder named after another account can still be attributed to that account. Return an explicit unknown/unowned state instead, or limit this path to trusted directories.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@orbit/pkg/table/ai_tools/internal/homes/owner_windows.go` around lines 7 -
10, Update the Windows statOwnerUID fallback path so owner() does not use
filepath.Base(dir) for attribution when ownership lookup fails. Return and
propagate an explicit unknown or unowned result on Windows, preserving
name-based attribution only for trusted directories if that existing mechanism
is available.
From the automated PR reviewers (Copilot, CodeRabbit); valid items only: Security: - fsutil.OpenRegular: add O_NOFOLLOW (unix, build-tagged; no-op on Windows) so the open itself refuses a final-component symlink, on top of the existing Lstat pre-check and post-open SameFile identity re-check. - jetbrains/gecko: open plugin .jar / .xpi archives through fsutil.OpenRegular + zip.NewReader instead of zip.OpenReader, so a symlinked/special file in a user-writable plugin dir can't be followed or block the root scan. - gecko: contain profiles.ini profile paths to the user's home (relative ".." and absolute escapes are refused). - instructions: read file Size via Lstat (not os.Stat), so a symlinked probe path can't leak a root-only file's size. Correctness: - apps.markRunning: match process names on word boundaries; short tokens like "dia"/"jan" no longer collide with substrings (e.g. "mediaanalysisd"). - tables: render MCP `enabled` as a label so an explicitly-disabled server (0) survives compactJSON instead of looking unset. - netsock: only classify ESTABLISHED (or status-less) connections; named transient states (TIME_WAIT/CLOSE_WAIT) are no longer treated as established. Robustness / cleanup: - instructions.scanContent: bounded io.ReadAll instead of a single Read that could short-read and truncate the scanned window. - tables.generate: short-circuit when the type constraint selects nothing. - tables.generate: honor ctx cancellation before the apps scan. - Fix package doc comment (ai_tools), the world_readable->world_writable risk comment, a stale smoke-test command path, and dead `_ = proj`. Adds regression tests for word-boundary process matching and profiles.ini containment. Deliberately not changed: the discriminator column stays `type` (confirmed in issue #47619; docs should reconcile to it, not the code); classify keeps skipping invalid embedded regexes rather than panicking in a root daemon's init; proc enumeration keeps its per-iteration cancellation check.
dantecatalfamo
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue: Resolves #47619
Adds a new
ai_toolstable to the fleetd tables extension so hosts running plain osquery (e.g. viaufa) get it too, not only hosts running the standalone extension from #47641.What this does
Vendors the
karmine05/agentic-detectorosquery extension (v0.3.0, commit7c942d0) intoorbit/pkg/table/ai_tools/by copying its source (internal/*collectors + thetablespackage, renamed toai_tools), and registers the unifiedai_toolstable cross-platform inOrbitDefaultTables().The table exposes a single schema with a
typediscriminator (mcp_server,ide_plugins,agents,apps,sockets,agent_instruction,browser_extension) plusrisk_flags,sha256, and a JSONdetailcolumn.WHERE type = '…'pushes down so only the requested collectors run.Dependencies (gopsutil/v4, howett.net/plist, gopkg.in/yaml.v3, osquery-go, golang.org/x/sys) already exist in
go.modat Fleet's pinned versions; onlyyaml.v3moved from indirect to direct. The upstream module is not added as a dependency — the source is vendored.Commits
Checklist for submitter
orbit/changes/.Testing
fleetd/orbit/Fleet Desktop
runtime.GOOSis used as needed to isolate changesSummary by CodeRabbit
ai_toolsinventory table covering AI apps, IDE plugins, browser extensions, agent tooling, MCP servers, instruction files, and related network/socket activity.