Add Chrome Remote Desktop detection script#50
Merged
Conversation
New rmm-misc/chrome-remote-desktop-detect.ps1 detects Chrome Remote Desktop installation or activity across all install vectors: - HKLM and HKCU uninstall registry keys (catches per-user Chrome installs that HKLM-only checks miss) - Program Files install path - %LOCALAPPDATA%\Google\Chrome Remote Desktop for every user profile - chromoting Windows service - remoting_host.exe process Writes 1 (active) or 0 (not active) to a configurable NinjaRMM custom field (default name "Remote"). Installation counts as active. Follows the standard DTC script template with RMM/interactive dual execution modes, transcript logging, and the Ninja-Property-Set call gated to RMM mode so interactive runs are safe.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, push a new commit or reopen this pull request to trigger a review.
Fits the app-{vendor-or-app} pattern better than rmm-misc, since Chrome
Remote Desktop is a Google Chrome family product and rmm-misc is a
catch-all that should not be the default home for app-specific scripts.
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 21 minutes and 43 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds two PowerShell detection scripts: a SYSTEM-context detector and a user-context detector that share per-tenant JSON state; scripts perform registry/file/service/process checks, log transcripts, update shared state, and conditionally write NinjaRMM custom fields. Changes
Sequence Diagram(s)sequenceDiagram
participant UserScript as User Detector
participant SharedJSON as Shared JSON (%PUBLIC%\{OrgName}\rmm-db\)
participant SystemScript as System Detector
participant NinjaRMM as NinjaRMM (Ninja-Property-Set)
participant Host as Host OS
UserScript->>Host: start transcript, read HKCU/LocalAppData
Host-->>UserScript: detection result
UserScript->>SharedJSON: read current state
UserScript->>SharedJSON: write/update user entry with timestamp
SharedJSON-->>UserScript: ack
UserScript->>Host: stop transcript, exit (0/1)
SystemScript->>Host: start transcript, validate OrgName/envs
SystemScript->>Host: perform HKLM, ProgramFiles, Service, Process checks
Host-->>SystemScript: check results
SystemScript->>SharedJSON: read user-context JSON
SharedJSON-->>SystemScript: return user entries
SystemScript->>SystemScript: compute detected/contextFoundIn/foundDetailsHtml
alt $env:RMM == "1"
SystemScript->>NinjaRMM: Ninja-Property-Set per-field (try/catch)
NinjaRMM-->>SystemScript: ack/fail
else
SystemScript->>Host: print would-write values
end
SystemScript->>Host: stop transcript, exit (detected)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
added 2 commits
April 7, 2026 13:30
More descriptive than the generic NinjaCustomField. Names the field by what it stores (Google Chrome [Remote Desktop] active state, boolean) rather than by the platform consuming it.
Drops HKU hive iteration and C:\Users profile iteration in favor of plain HKCU and $env:LOCALAPPDATA checks. Script is deployed in three contexts (daily from SYSTEM, at boot from SYSTEM, at user login as the user) and the user-login run is what catches per-user Chrome installs. SYSTEM runs still cover system-wide installs and any active service or process.
![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: 2
🧹 Nitpick comments (1)
app-google-chrome/chrome-remote-desktop-detect.ps1 (1)
239-240: Exit code semantics are inverted from typical convention.Using
exit 1to indicate "active/found" andexit 0for "not found" inverts the standard success=0 convention. This is documented in the header comments (line 26), but verify that downstream RMM alerting/automation handles this correctly—some RMM platforms may interpret exit code 1 as a script failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-google-chrome/chrome-remote-desktop-detect.ps1` around lines 239 - 240, The script currently uses exit $result where $result maps 1="found/active" and 0="not found", inverting the usual success=0 convention; change the exit semantics so that a successful detection (resource found/active) returns exit 0 and a non-detection returns a non-zero code (e.g., 1), update the logic around the exit invocation (the Stop-Transcript / exit $result block) to map detection boolean to these standard codes, and update the header comment describing exit semantics to match the new convention; ensure any references to the variable name (e.g., $result) or function that sets it are adjusted accordingly so downstream RMM interprets the exit code correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app-google-chrome/chrome-remote-desktop-detect.ps1`:
- Around line 156-170: Test-CRDUserAppData currently hardcodes "C:\Users" and
will miss relocated profile folders; update it to enumerate user profiles from
the system profile registry (HKLM:\SOFTWARE\Microsoft\Windows
NT\CurrentVersion\ProfileList) or via Win32_UserProfile (Get-CimInstance
Win32_UserProfile) to read each ProfileImagePath, expand any environment
variables and skip built-in/system SIDs, then join "AppData\Local\Google\Chrome
Remote Desktop" to each resolved profile path and Test-Path as before (preserve
the existing Write-Host message and return behavior). Ensure you still filter
out Default/Public/Default User/All Users equivalents by name or by well-known
SIDs when using registry/CIM.
- Around line 227-237: The Ninja-Property-Set call in the RMM branch uses named
parameters (-Name, -Value) but the cmdlet expects positional parameters; update
the call inside the if ($RMM -eq 1) try block to call Ninja-Property-Set with
positional arguments (first the $NinjaCustomField, then $result) and keep the
existing try/catch and logging around it so the Write-Host success and error
messages still reference $NinjaCustomField and $result.
---
Nitpick comments:
In `@app-google-chrome/chrome-remote-desktop-detect.ps1`:
- Around line 239-240: The script currently uses exit $result where $result maps
1="found/active" and 0="not found", inverting the usual success=0 convention;
change the exit semantics so that a successful detection (resource found/active)
returns exit 0 and a non-detection returns a non-zero code (e.g., 1), update the
logic around the exit invocation (the Stop-Transcript / exit $result block) to
map detection boolean to these standard codes, and update the header comment
describing exit semantics to match the new convention; ensure any references to
the variable name (e.g., $result) or function that sets it are adjusted
accordingly so downstream RMM interprets the exit code correctly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7abf6608-c8f7-4e41-9018-a07dbff101ce
📒 Files selected for processing (1)
app-google-chrome/chrome-remote-desktop-detect.ps1
Comment on lines
+156
to
+170
| function Test-CRDUserAppData { | ||
| $found = $false | ||
| $userProfiles = Get-ChildItem "C:\Users" -Directory -ErrorAction SilentlyContinue | Where-Object { | ||
| $_.Name -notin @('Public', 'Default', 'Default User', 'All Users') | ||
| } | ||
|
|
||
| foreach ($userProfile in $userProfiles) { | ||
| $crdPath = Join-Path $userProfile.FullName "AppData\Local\Google\Chrome Remote Desktop" | ||
| if (Test-Path $crdPath) { | ||
| Write-Host " [Profile] Found in $($userProfile.Name): $crdPath" | ||
| $found = $true | ||
| } | ||
| } | ||
| return $found | ||
| } |
Contributor
There was a problem hiding this comment.
Hardcoded "C:\Users" path may miss relocated profiles.
Windows allows the default profile directory to be relocated (e.g., D:\Users). Consider using the registry or $env:SystemDrive for better portability.
Suggested fix using ProfileList registry
function Test-CRDUserAppData {
$found = $false
- $userProfiles = Get-ChildItem "C:\Users" -Directory -ErrorAction SilentlyContinue | Where-Object {
- $_.Name -notin @('Public', 'Default', 'Default User', 'All Users')
- }
+ # Query ProfileList for actual profile paths (handles relocated profiles)
+ $profileListPath = "HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList"
+ $userProfiles = Get-ChildItem $profileListPath -ErrorAction SilentlyContinue | ForEach-Object {
+ $profilePath = (Get-ItemProperty $_.PSPath -ErrorAction SilentlyContinue).ProfileImagePath
+ if ($profilePath -and (Test-Path $profilePath)) {
+ Get-Item $profilePath -ErrorAction SilentlyContinue
+ }
+ } | Where-Object {
+ $_.Name -notin @('Public', 'Default', 'Default User', 'All Users', 'systemprofile', 'LocalService', 'NetworkService')
+ }
foreach ($userProfile in $userProfiles) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-google-chrome/chrome-remote-desktop-detect.ps1` around lines 156 - 170,
Test-CRDUserAppData currently hardcodes "C:\Users" and will miss relocated
profile folders; update it to enumerate user profiles from the system profile
registry (HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList) or via
Win32_UserProfile (Get-CimInstance Win32_UserProfile) to read each
ProfileImagePath, expand any environment variables and skip built-in/system
SIDs, then join "AppData\Local\Google\Chrome Remote Desktop" to each resolved
profile path and Test-Path as before (preserve the existing Write-Host message
and return behavior). Ensure you still filter out Default/Public/Default
User/All Users equivalents by name or by well-known SIDs when using
registry/CIM.
Comment on lines
+227
to
+237
| if ($RMM -eq 1) { | ||
| try { | ||
| Ninja-Property-Set -Name $NinjaCustomField -Value $result | ||
| Write-Host "Wrote $result to NinjaRMM custom field '$NinjaCustomField'" | ||
| } catch { | ||
| Write-Host "ERROR: Failed to write to NinjaRMM custom field '$NinjaCustomField' - $_" | ||
| } | ||
| } else { | ||
| Write-Host "Interactive mode - skipping Ninja-Property-Set call" | ||
| Write-Host "Would have written: Ninja-Property-Set -Name '$NinjaCustomField' -Value $result" | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
NinjaRMM Ninja-Property-Set PowerShell cmdlet syntax parameters
💡 Result:
Ninja-Property-Set (NinjaOne / NinjaRMM) PowerShell command
Syntax (positional arguments):
Ninja-Property-Set <fieldName> <value>Ninja-Property-Set is used to set a NinjaOne custom field to a specific value. [1]
Parameters (positional):
<fieldName>– The custom field Name (not the label) you want to write to. [1]<value>– The value to assign (format depends on the field type; use quotes for values containing spaces). [1]
Examples
Ninja-Property-Set domainjoined 1
Ninja-Property-Set domainjoined 0[1]
Multi-line text example (use `n to embed newlines; wrap in quotes):
Ninja-Property-Set multiline "line with spaces`nline2`nline3`nfinalval"[2]
Note: NinjaOne’s newer PowerShell functions are Get-NinjaProperty / Set-NinjaProperty (described as evolutions of Ninja-Property-Get / Ninja-Property-Set). If you meant the full parameterized cmdlet-style syntax, it’s typically documented under Set-NinjaProperty instead. [2]
Sources: [1] [2]
Fix Ninja-Property-Set syntax to use positional parameters.
The Ninja-Property-Set cmdlet expects positional arguments, not named -Name and -Value parameters. The current syntax will fail.
Suggested fix
if ($RMM -eq 1) {
try {
- Ninja-Property-Set -Name $NinjaCustomField -Value $result
+ Ninja-Property-Set $NinjaCustomField $result
Write-Host "Wrote $result to NinjaRMM custom field '$NinjaCustomField'"
} catch {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-google-chrome/chrome-remote-desktop-detect.ps1` around lines 227 - 237,
The Ninja-Property-Set call in the RMM branch uses named parameters (-Name,
-Value) but the cmdlet expects positional parameters; update the call inside the
if ($RMM -eq 1) try block to call Ninja-Property-Set with positional arguments
(first the $NinjaCustomField, then $result) and keep the existing try/catch and
logging around it so the Write-Host success and error messages still reference
$NinjaCustomField and $result.
The combined script had to apologize for half its checks in either
context. Splitting it into two purpose-built scripts:
chrome-remote-desktop-detect-system.ps1
- Runs from SYSTEM (daily, boot)
- HKLM, Program Files, chromoting service, remoting_host process
- Logs to $ENV:WINDIR\logs
- Default custom field: Remote
chrome-remote-desktop-detect-user.ps1
- Runs in user context (login)
- HKCU and %LOCALAPPDATA% only
- Logs to $ENV:LOCALAPPDATA\dtc-logs (user-writable)
- Default custom field: RemoteUser
The two scripts default to different custom field names so they don't
overwrite each other. OR them together in NinjaRMM dashboards or
conditions to get a single 'CRD anywhere' signal.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app-google-chrome/chrome-remote-desktop-detect.ps1 (1)
35-35: Naming style is inconsistent with repo PowerShell rule.Variables/functions in this file use mixed casing (e.g.,
ScriptLogName,Test-CRDInstalledHKLM) rather than camelCase. Consider normalizing to reduce style drift across scripts.As per coding guidelines, "Use camelCase for PowerShell variables and functions".
Also applies to: 90-179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-google-chrome/chrome-remote-desktop-detect.ps1` at line 35, Variables and function names in this script use mixed/Pascal casing (e.g., ScriptLogName, Test-CRDInstalledHKLM) which violates the repo rule to use camelCase; rename all top-level variables and function identifiers to camelCase (for example ScriptLogName -> scriptLogName, Test-CRDInstalledHKLM -> test-CrdInstalledHklm or testCrdInstalledHklm depending on your function naming convention), and update every reference/callsite within the file (including any invocations, argument passing, and exported/used symbols) to the new names to ensure consistency across the file (also apply the same changes to other identifiers in the 90-179 region mentioned).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app-google-chrome/chrome-remote-desktop-detect.ps1`:
- Around line 51-57: The checks currently only test for $null so empty strings
slip through; update the conditionals to use robust string emptiness checks
(e.g. [string]::IsNullOrWhiteSpace) for $RMMScriptPath and $Description so empty
or whitespace-only values trigger the fallback: when $RMMScriptPath is not
null/empty/whitespace set $LogPath = "$RMMScriptPath\logs\$ScriptLogName",
otherwise set $LogPath = "$ENV:WINDIR\logs\$ScriptLogName" (this preserves the
RMM fallback to %WINDIR%\logs), and change the $Description check to use the
same IsNullOrWhiteSpace test before writing the "Description is null..."
message.
---
Nitpick comments:
In `@app-google-chrome/chrome-remote-desktop-detect.ps1`:
- Line 35: Variables and function names in this script use mixed/Pascal casing
(e.g., ScriptLogName, Test-CRDInstalledHKLM) which violates the repo rule to use
camelCase; rename all top-level variables and function identifiers to camelCase
(for example ScriptLogName -> scriptLogName, Test-CRDInstalledHKLM ->
test-CrdInstalledHklm or testCrdInstalledHklm depending on your function naming
convention), and update every reference/callsite within the file (including any
invocations, argument passing, and exported/used symbols) to the new names to
ensure consistency across the file (also apply the same changes to other
identifiers in the 90-179 region mentioned).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c4ccb49-c18a-405b-82d8-9afce80d8129
📒 Files selected for processing (1)
app-google-chrome/chrome-remote-desktop-detect.ps1
Comment on lines
+51
to
+57
| if ($null -ne $RMMScriptPath) { | ||
| $LogPath = "$RMMScriptPath\logs\$ScriptLogName" | ||
| } else { | ||
| $LogPath = "$ENV:WINDIR\logs\$ScriptLogName" | ||
| } | ||
| if ($null -eq $Description) { | ||
| Write-Host "Description is null. This was most likely run automatically from the RMM." |
Contributor
There was a problem hiding this comment.
Harden RMM fallback checks for empty values.
Line 51 and Line 56 only test for $null, so empty strings can bypass fallback/default behavior and produce weak paths/descriptions.
Suggested fix
- if ($null -ne $RMMScriptPath) {
+ if (-not [string]::IsNullOrWhiteSpace($RMMScriptPath)) {
$LogPath = "$RMMScriptPath\logs\$ScriptLogName"
} else {
$LogPath = "$ENV:WINDIR\logs\$ScriptLogName"
}
- if ($null -eq $Description) {
+ if ([string]::IsNullOrWhiteSpace($Description)) {
Write-Host "Description is null. This was most likely run automatically from the RMM."
$Description = "RMM Automated Scan"
}As per coding guidelines, "Input handling must detect $RMM ... set $LogPath to %WINDIR%\logs in interactive, and $RMMScriptPath\logs (fallback to %WINDIR%\logs) in RMM".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-google-chrome/chrome-remote-desktop-detect.ps1` around lines 51 - 57, The
checks currently only test for $null so empty strings slip through; update the
conditionals to use robust string emptiness checks (e.g.
[string]::IsNullOrWhiteSpace) for $RMMScriptPath and $Description so empty or
whitespace-only values trigger the fallback: when $RMMScriptPath is not
null/empty/whitespace set $LogPath = "$RMMScriptPath\logs\$ScriptLogName",
otherwise set $LogPath = "$ENV:WINDIR\logs\$ScriptLogName" (this preserves the
RMM fallback to %WINDIR%\logs), and change the $Description check to use the
same IsNullOrWhiteSpace test before writing the "Description is null..."
message.
added 3 commits
April 7, 2026 13:41
Both detection scripts now contribute to a single text custom field (default 'RemoteContext') in addition to their boolean fields. Each script reads the current value, drops or adds its own context label, and writes the merged result back, so the field always reflects the union of both signals: '', 'System', 'User', or 'System, User'. Read-merge-write avoids the clobber problem ... the system script's 'System' label survives a user-context login on a machine where the user's HKCU does not have CRD, and vice versa. New RMM variable $CustomFieldGoogleChromeContextString is shared between both scripts and defaults to the same field name in each.
NinjaRMM passes script preset variables as environment variables, so $RMM, $Description, $RMMScriptPath, and the two custom field names all need to be accessed via $env:VarName. Both scripts now reference $env: at every use site so the source is obvious from any line. Defaults for the custom field name variables are set at the top by writing to $env: directly, so the rest of the script can keep using the $env: form consistently. Also fixed an inverted null check inherited from script-template- powershell.ps1 that read 'if RMMScriptPath is null, build the log path from $RMMScriptPath' (which would have produced a path starting with a backslash). The check is now '-not IsNullOrEmpty'. NOTE: This diverges from the documented repo convention in CLAUDE.md and script-template-powershell.ps1, both of which use bare $RMM. The existing pattern only works in true RMM mode if NinjaRMM also injects preset variables as PowerShell session variables, which is unverified. Worth a separate cleanup pass on the template and existing scripts.
…mdlets
Reported failure from a real user-context run:
Failed to start ninjarmm-cli.
Unable to find ninjarmm-cli.exe.
Updated NinjaRMM context field 'googleChromeRemoteDesktopContextFoundIn':
'Unable to find ninjarmm-cli.exe.' -> 'Unable to find ninjarmm-cli.exe.'
The Ninja-Property-Get and Ninja-Property-Set cmdlets only work from
SYSTEM context (they shell out to ninjarmm-cli.exe which is in a
SYSTEM-only path). When called from user context they printed the
error string to stdout, which the read-merge-write helper happily
captured as the existing field value and 'wrote back' to the same
broken value.
Architecture change:
- User-context script no longer calls any Ninja cmdlets. It writes
a presence-only state file at
$env:GoogleChromeRemoteDesktopStateDir\:USERNAME.txt when CRD
is detected, removes it when not. File mtime is the timestamp.
- System-context script is now the SOLE writer to NinjaRMM. It runs
its own system-side checks (HKLM, Program Files, service, process)
and ALSO reads the per-user state file directory, aggregating
both signals into a single boolean and a context string.
- System script purges stale state files older than
$env:GoogleChromeRemoteDesktopStateMaxAgeDays (default 90).
- System script defensively checks WindowsIdentity.IsSystem before
calling Ninja cmdlets, so a misconfigured deployment falls back
to logging instead of writing garbage to the custom field.
- The Update-CRDContextField read-merge-write helper is gone.
Single writer means single source of truth, no race conditions,
no merge logic needed.
Default state directory: C:\ProgramData\DTC\google-chrome-remote-desktop-state
ProgramData allows authenticated users to create files with inherited
permissions, so each user can write their own state file without
needing the directory pre-provisioned with custom ACLs.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
app-google-chrome/chrome-remote-desktop-detect-system.ps1 (1)
228-240:⚠️ Potential issue | 🟠 MajorUse the supported Ninja field-write syntax.
Both
Ninja-Property-Setcalls still use-Name/-Value. If the agent only exposes the documented positional form, the boolean write and the shared-context update will both fail in RMM mode. Please make the same change in the companion user script.Proposed fix
- Ninja-Property-Set -Name $FieldName -Value $newValue + Ninja-Property-Set $FieldName $newValue @@ - Ninja-Property-Set -Name $env:CustomFieldGoogleChromeActiveBoolean -Value $result + Ninja-Property-Set $env:CustomFieldGoogleChromeActiveBoolean $resultWhat is the documented PowerShell syntax for NinjaOne/NinjaRMM `Ninja-Property-Set`? Does it support `-Name` / `-Value`, or only positional arguments like `Ninja-Property-Set <fieldName> <value>`?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-google-chrome/chrome-remote-desktop-detect-system.ps1` around lines 228 - 240, The Ninja-Property-Set calls are using named parameters (-Name / -Value) which the agent may not support; replace both occurrences so they call Ninja-Property-Set with positional arguments (field name first, value second) instead of -Name/-Value — update the call that currently uses Ninja-Property-Set -Name $FieldName -Value $newValue (the shared-context update in the try/catch) and the call that uses Ninja-Property-Set -Name $env:CustomFieldGoogleChromeActiveBoolean -Value $result (the RMM-only boolean write), and make the identical change in the companion user script so both use Ninja-Property-Set <fieldName> <value> style invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app-google-chrome/chrome-remote-desktop-detect-system.ps1`:
- Around line 207-233: The read-modify-write in the RemoteContext merge (uses
Ninja-Property-Get, Ninja-Property-Set, FieldName, ThisContext, IsActive) is not
atomic and races between the boot and user scripts; fix by making the update
atomic or eliminating concurrent writers: either (A) switch to separate
per-context fields (e.g., RemoteContext_System and RemoteContext_User) so each
script only writes its own key, or (B) implement a compare-and-swap retry loop
in both chrome-remote-desktop-detect-system.ps1 and
chrome-remote-desktop-detect-user.ps1 that reads current via Ninja-Property-Get,
computes newValue, then attempts to write only if the current value is unchanged
(retry on mismatch/backoff), or alternatively designate one authoritative writer
for merging and have the other script only set a flag the authoritative writer
consumes.
In `@app-google-chrome/chrome-remote-desktop-detect-user.ps1`:
- Around line 110-160: The detection currently only checks the active user's
hive (Test-CRDInstalledHKCU and Test-CRDUserAppData) and then sets a
machine-level result ($isActive/$result), which can clear a device-level "user"
finding when a different profile on the same device still has CRD; update the
logic to aggregate across all local user profiles instead of using only the
current HKCU/LOCALAPPDATA: iterate relevant user SIDs under HKU and each user's
LocalAppData folders to call the equivalent checks (or a helper that accepts a
hive/path) and treat the machine as active if any profile yields true, ensuring
$isActive/$result is derived from the aggregated per-profile results rather than
a single logged-in user scan (modify Test-CRDInstalledHKCU/Test-CRDUserAppData
or add a new enumerator function to perform profile-wide checks).
- Around line 60-76: The script currently writes logs to $env:LOCALAPPDATA for
both interactive and RMM paths; change it to follow the template contract: keep
the Read-Host validation loop and set/ensure $env:Description is captured, then
set $LogPath to "$env:WINDIR\logs\$ScriptLogName" in the interactive branch
(when $env:RMM -ne "1"); in the RMM branch (when $env:RMM -eq "1") set $LogPath
to "$env:RMMScriptPath\logs\$ScriptLogName" but fall back to
"$env:WINDIR\logs\$ScriptLogName" if $env:RMMScriptPath is null/empty; update
references to $env:LOCALAPPDATA to $env:WINDIR only for these log path
assignments while keeping the existing Read-Host loop and $env:Description
handling.
---
Duplicate comments:
In `@app-google-chrome/chrome-remote-desktop-detect-system.ps1`:
- Around line 228-240: The Ninja-Property-Set calls are using named parameters
(-Name / -Value) which the agent may not support; replace both occurrences so
they call Ninja-Property-Set with positional arguments (field name first, value
second) instead of -Name/-Value — update the call that currently uses
Ninja-Property-Set -Name $FieldName -Value $newValue (the shared-context update
in the try/catch) and the call that uses Ninja-Property-Set -Name
$env:CustomFieldGoogleChromeActiveBoolean -Value $result (the RMM-only boolean
write), and make the identical change in the companion user script so both use
Ninja-Property-Set <fieldName> <value> style invocation.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 980f6add-1c39-4dd1-b73e-33622da1edd0
📒 Files selected for processing (2)
app-google-chrome/chrome-remote-desktop-detect-system.ps1app-google-chrome/chrome-remote-desktop-detect-user.ps1
Comment on lines
+60
to
+76
| if ($env:RMM -ne "1") { | ||
| $ValidInput = 0 | ||
| while ($ValidInput -ne 1) { | ||
| $env:Description = Read-Host "Please enter the ticket # and/or your initials for audit trail" | ||
| if ($env:Description) { | ||
| $ValidInput = 1 | ||
| } else { | ||
| Write-Host "Invalid input. Please try again." | ||
| } | ||
| } | ||
| $LogPath = "$env:LOCALAPPDATA\dtc-logs\$ScriptLogName" | ||
| } else { | ||
| if (-not [string]::IsNullOrEmpty($env:RMMScriptPath)) { | ||
| $LogPath = "$env:RMMScriptPath\logs\$ScriptLogName" | ||
| } else { | ||
| $LogPath = "$env:LOCALAPPDATA\dtc-logs\$ScriptLogName" | ||
| } |
Contributor
There was a problem hiding this comment.
Keep the user script on the standard transcript path contract.
Interactive mode and the RMM fallback now write under %LOCALAPPDATA%\dtc-logs, but the PowerShell template requires %WINDIR%\logs for interactive runs and $RMMScriptPath\logs with a %WINDIR%\logs fallback in RMM mode. This divergence will break the repo's documented log discovery and RMM-simulation workflow unless the template is changed at the same time. As per coding guidelines, "Input handling must detect $RMM (1 for RMM, undefined for interactive); interactive uses Read-Host with validation loop; RMM uses pre-set variables; always capture $Description; set $LogPath to %WINDIR%\logs in interactive, and $RMMScriptPath\logs (fallback to %WINDIR%\logs) in RMM".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-google-chrome/chrome-remote-desktop-detect-user.ps1` around lines 60 -
76, The script currently writes logs to $env:LOCALAPPDATA for both interactive
and RMM paths; change it to follow the template contract: keep the Read-Host
validation loop and set/ensure $env:Description is captured, then set $LogPath
to "$env:WINDIR\logs\$ScriptLogName" in the interactive branch (when $env:RMM
-ne "1"); in the RMM branch (when $env:RMM -eq "1") set $LogPath to
"$env:RMMScriptPath\logs\$ScriptLogName" but fall back to
"$env:WINDIR\logs\$ScriptLogName" if $env:RMMScriptPath is null/empty; update
references to $env:LOCALAPPDATA to $env:WINDIR only for these log path
assignments while keeping the existing Read-Host loop and $env:Description
handling.
added 2 commits
April 7, 2026 14:25
Reverts the over-engineered ProgramData per-user state file approach
in favor of what was actually asked for: a single shared JSON at
C:\Users\Public\DTC\rmm-db\google-chrome-remote-desktop-user-active.json
mapping username -> ISO timestamp.
User script:
- Detects HKCU + %LOCALAPPDATA%
- Reads the JSON, adds/updates own entry if active, removes own
entry if not, writes JSON back
- No NinjaRMM cmdlet calls. No staleness checks. No defensive
IsSystem checks.
System script (sole writer to NinjaRMM):
- Detects HKLM + Program Files + chromoting service +
remoting_host process
- Reads the JSON to learn which users are active
- Writes 3 NinjaRMM custom fields:
googleChromeRemoteDesktopDetected (boolean 1/0)
googleChromeRemoteDesktopContextFoundIn (text:
'System' / 'User' / 'User + System')
googleChromeRemoteDesktopFoundDetails (HTML: pretty list of
system hits and active usernames with last-seen timestamps)
All custom field names are configurable via $env: variables for
deployment flexibility. Defaults match the camelCase naming convention
already in use.
Script changes:
- Add required $env:OrgName variable to both CRD scripts. Hard-fail
in RMM mode if missing, prompt interactively. Used to namespace
the shared state file under %PUBLIC%\<OrgName>\rmm-db\ for
white-label deployability across MSPs.
- Drop the now-redundant $env:GoogleChromeRemoteDesktopUserStatePath
override variable. The path is computed once at the top into a
local $UserStatePath from $env:PUBLIC + $env:OrgName.
- User-context script also namespaces its log directory by OrgName:
$env:LOCALAPPDATA\:OrgName-logs
CLAUDE.md update:
- Document the $env: convention for reading RMM-supplied variables
(NinjaRMM passes presets as environment variables, bare $RMM
silently fails in true RMM mode)
- New 'Application Detection Patterns' section covering all six
install/runtime vectors (HKLM, HKCU, Program Files, %LOCALAPPDATA%,
Windows service, running process)
- New 'NinjaRMM Custom Field Patterns' section covering field types
(Boolean, Text, HTML, Multi-line) and the SYSTEM-only Ninja-Property
cmdlet caveat
- New 'Cross-Context Detection (User + System Split)' section
documenting the dual-script + shared JSON pattern, the
$env:PUBLIC\:OrgName\rmm-db\ convention, JSON-vs-SQLite
rationale, and required $env:OrgName variable
- Updated 'Testing Scripts' section to use $env:RMM=1 instead of
bare $RMM=1
- Bundled with the existing in-progress CLAUDE.md cleanup that was
sitting in working tree (verbose Git Workflow section removed in
favor of the condensed version)
README.md rewrite:
- Expanded from minimal stub to a proper entry-point document
- Quick start for both interactive and RMM execution
- Repository structure table
- Script conventions including the $env: pattern
- Cross-context detection pattern with the canonical CRD example
- Field type table for NinjaRMM custom fields
- Points to CLAUDE.md and CONTRIBUTING.md for deeper details
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app-google-chrome/chrome-remote-desktop-detect-user.ps1 (1)
215-220: Consider the impact of failed state file writes.If
Write-CRDUserStatefails, the script logs the error but still exits with the detection result ($result). This means the system script won't see this user's activity even though CRD was detected. The current behavior is acceptable for logging purposes, but consider whether a write failure should be more prominently flagged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-google-chrome/chrome-remote-desktop-detect-user.ps1` around lines 215 - 220, The write of the CRD user state file may fail silently while the script still returns the detection result; update the catch block around Write-CRDUserState (referencing Write-CRDUserState, $UserStatePath, $state, and $result) to escalate the failure: log the error as now, then set a distinct failure indicator (e.g. set $result to a non-zero/error value or set a global flag like $CRDStateWriteFailed) and exit with a non-zero exit code (or otherwise surface the failure to the caller) so callers can detect that state persistence failed.README.md (1)
132-139: Add language specifiers to fenced code blocks.Markdownlint flags these code blocks as missing a language identifier. Since they display file paths rather than executable code, use
textorplaintextto satisfy linters and signal intent.📝 Suggested fix
-``` +```text category/<thing>-detect-system.ps1 # runs as SYSTEM, daily + boot category/<thing>-detect-user.ps1 # runs in user context, at loginShared state file:
-+text
$env:PUBLIC$env:OrgName\rmm-db<thing>-user-active.json🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 132 - 139, The fenced code blocks in the README lack language specifiers which markdownlint flags; update both code fences to include a language like "text" (or "plaintext") so they read as ```text ... ``` for the block containing "category/<thing>-detect-system.ps1 # runs as SYSTEM, daily + boot" / "category/<thing>-detect-user.ps1 # runs in user context, at login" and for the Shared state file block containing "$env:PUBLIC\$env:OrgName\rmm-db\<thing>-user-active.json".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app-google-chrome/chrome-remote-desktop-detect-user.ps1`:
- Around line 203-205: The timestamp currently uses local time but appends a 'Z'
(UTC) suffix via Get-Date -Format 'yyyy-MM-ddTHH:mm:ssZ'; change the timestamp
generation in the $isActive block so it emits an actual UTC timestamp (e.g.,
convert the current time to UTC and format it as ISO 8601 with the trailing 'Z')
before assigning to $state[$env:USERNAME] and writing the host message; update
the Get-Date usage referenced in that block to use a UTC conversion (or
PowerShell's UTC option) so the 'Z' suffix is accurate.
---
Nitpick comments:
In `@app-google-chrome/chrome-remote-desktop-detect-user.ps1`:
- Around line 215-220: The write of the CRD user state file may fail silently
while the script still returns the detection result; update the catch block
around Write-CRDUserState (referencing Write-CRDUserState, $UserStatePath,
$state, and $result) to escalate the failure: log the error as now, then set a
distinct failure indicator (e.g. set $result to a non-zero/error value or set a
global flag like $CRDStateWriteFailed) and exit with a non-zero exit code (or
otherwise surface the failure to the caller) so callers can detect that state
persistence failed.
In `@README.md`:
- Around line 132-139: The fenced code blocks in the README lack language
specifiers which markdownlint flags; update both code fences to include a
language like "text" (or "plaintext") so they read as ```text ... ``` for the
block containing "category/<thing>-detect-system.ps1 # runs as SYSTEM, daily +
boot" / "category/<thing>-detect-user.ps1 # runs in user context, at login"
and for the Shared state file block containing
"$env:PUBLIC\$env:OrgName\rmm-db\<thing>-user-active.json".
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2221d824-6d30-43a1-830e-0ac3caf4ade5
📒 Files selected for processing (4)
CLAUDE.mdREADME.mdapp-google-chrome/chrome-remote-desktop-detect-system.ps1app-google-chrome/chrome-remote-desktop-detect-user.ps1
✅ Files skipped from review due to trivial changes (2)
- CLAUDE.md
- app-google-chrome/chrome-remote-desktop-detect-system.ps1
Comment on lines
+203
to
+205
| if ($isActive) { | ||
| $state[$env:USERNAME] = (Get-Date -Format 'yyyy-MM-ddTHH:mm:ssZ') | ||
| Write-Host "Adding/updating $env:USERNAME in user state file" |
Contributor
There was a problem hiding this comment.
Timestamp uses local time with misleading 'Z' suffix.
Line 204 uses Get-Date -Format 'yyyy-MM-ddTHH:mm:ssZ' which appends a literal 'Z' to local time. The 'Z' suffix in ISO 8601 specifically indicates UTC, but Get-Date returns local time by default. While the system script only displays these timestamps and doesn't perform time calculations, this could cause confusion in multi-timezone environments.
🔧 Suggested fix to use actual UTC
if ($isActive) {
- $state[$env:USERNAME] = (Get-Date -Format 'yyyy-MM-ddTHH:mm:ssZ')
+ $state[$env:USERNAME] = (Get-Date).ToUniversalTime().ToString('yyyy-MM-ddTHH:mm:ssZ')
Write-Host "Adding/updating $env:USERNAME in user state file"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-google-chrome/chrome-remote-desktop-detect-user.ps1` around lines 203 -
205, The timestamp currently uses local time but appends a 'Z' (UTC) suffix via
Get-Date -Format 'yyyy-MM-ddTHH:mm:ssZ'; change the timestamp generation in the
$isActive block so it emits an actual UTC timestamp (e.g., convert the current
time to UTC and format it as ISO 8601 with the trailing 'Z') before assigning to
$state[$env:USERNAME] and writing the host message; update the Get-Date usage
referenced in that block to use a UTC conversion (or PowerShell's UTC option) so
the 'Z' suffix is accurate.
The previous template documented and exemplified the bare $RMM /
$Description / $RMMScriptPath pattern, which silently fails in true
NinjaRMM execution because NinjaRMM passes preset variables as
environment variables (not PowerShell session variables). The bare
references resolve to $null and the script falls through to the
interactive branch, then bails when there is no TTY for Read-Host.
Template now:
- Reads all RMM-supplied variables via $env: at every use site
- Compares $env:RMM against the string "1" (env vars are strings)
- Documents the variable comment block convention with $env: prefix
- Includes a commented-out example of the optional variable default
pattern (write back to $env: so the rest of the script can keep
using the $env: form consistently)
- Mentions $env:OrgName as a required variable for cross-context
scripts that share state with a user-context companion
- Creates the log directory before Start-Transcript so log writes
do not silently fail on a fresh box
- Fixes the inverted null check inherited from the old template
(was 'if RMMScriptPath is null, build path from $RMMScriptPath'
which produced a path starting with a backslash)
- Fixes the 'VARIALBES' typo in the variable header comment
- Points to CLAUDE.md for the full pattern documentation
Gumbees
commented
May 11, 2026
Gumbees
left a comment
Gumbees
left a comment
Contributor
Author
There was a problem hiding this comment.
Summary
Adds a Chrome Remote Desktop cross-context detection pair (system + user) with shared JSON state under %PUBLIC%\<OrgName>\rmm-db\, plus substantial CLAUDE.md and template updates documenting the env-var RMM variable convention and the cross-context detection pattern.
What works
- The two-script pattern (
*-detect-system.ps1runs as SYSTEM withNinja-Property-Set;*-detect-user.ps1runs in user context, writes JSON, never touches Ninja cmdlets) is the correct architecture. SYSTEM can't see HKCU or%LOCALAPPDATA%andNinja-Property-Setonly works as SYSTEM ... documenting and enforcing the split is right. $env:OrgNamenamespacing under%PUBLIC%is shell-friendly and avoids ACL surprises ... user processes can write, SYSTEM can read.- Detection coverage is broad: HKLM + HKCU uninstall (32 and 64 bit), Program Files,
%LOCALAPPDATA%,chromotingservice,remoting_host.exeprocess. The original sample missed the HKCU/LOCALAPPDATA per-user case which is the common install path for CRD. - CLAUDE.md additions on
Ninja-Property-Setonly working as SYSTEM (and silently returning the error string fromNinja-Property-Get) is a real footgun ... worth documenting at the canon level. app-google-chrome/is a reasonable new folder under theapp-*convention.
Concerns
- The CLAUDE.md change pivots the canonical pattern from
$RMMto$env:RMM(string"1", not integer). That's a meaningful contract change for the rest of the repo ... PR #52 just landed using the integer/non-env pattern, and most existing scripts use$RMM -ne 1. Either the existing scripts get a sweep to convert, or the doc needs to clarify "new scripts use$env:convention, existing scripts keep$RMMuntil touched." Halfway-applied standards are worse than none. - The Git Workflow section was removed from CLAUDE.md. Intentional? It's still valuable context for new contributors and Claude Code sessions ... if it moved somewhere else (CONTRIBUTING.md?) make sure the link survives.
- The 1064-line diff includes both the new scripts and a substantial CLAUDE.md / template rewrite. Mechanically this is two PRs glued together. Not blocking, but a future split would make each easier to review and revert independently if needed.
- Once the env-var convention is canon, the existing template (
script-template-powershell.ps1) needs to ship matching shape, or new contributors will copy stale boilerplate. Make sure the template in this PR is the canonical template moving forward.
Suggestions
- Consider a one-line "deprecated, use env-var pattern" comment near the old
$RMMexample in the README so existing scripts don't get cargo-culted forward. - The HTML field generator using
StringBuilderis nice ... worth lifting into a tiny shared helper ins3-api-lib/or a newrmm-ninja-helpers.ps1if more scripts adopt it. - Add a
Ninja-Property-Setwrapper that no-ops outside RMM mode (the pattern in PR #43'sMonitor-InstallerPatchesuses try/catch for this) so dev runs don't error.
Verdict
COMMENT ... no blockers, scripts are solid and the documentation is genuinely useful. Main thing is the env-var convention pivot has knock-on implications for the rest of the repo that should be planned explicitly rather than left to drift.
This was referenced May 11, 2026
This was referenced May 11, 2026
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
1 participant
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.
Summary
New
rmm-misc/chrome-remote-desktop-detect.ps1detects whether Chrome Remote Desktop is installed or active on an endpoint and writes1or0to a NinjaRMM custom field.Built from sample code, then aligned with the DTC script template (
script-template-powershell.ps1) and modeled onmsft-office/cve-2026-21509-detect.ps1for structure.What it checks
The sample only looked at HKLM uninstall keys and the
remoting_hostprocess, which misses the most common case: Chrome Remote Desktop installed per-user via the Chrome browser, which lives inHKCUand%LOCALAPPDATA%. This script covers all of:Program Files\Google\Chrome Remote Desktopinstall path%LOCALAPPDATA%\Google\Chrome Remote Desktopfor every user profile on the boxchromotingWindows service (Chrome Remote Desktop Service)remoting_host.exeprocessIf any check returns true, the result is
1. Otherwise0. Installation counts as active per the requirement.Output
1or0to a NinjaRMM custom field (default nameRemote, configurable via$NinjaCustomFieldRMM variable)$ENV:WINDIR\logs\chrome-remote-desktop-detect.log(or$RMMScriptPath\logs\when run from RMM)0not active,1active)Template alignment
## $VarNameRMM variable comment block at the top$RMM-aware dual execution mode with$Descriptionaudit trailStart-TranscriptStart-Transcript/Stop-Transcriptwrapping all logicTest-CRD*Ninja-Property-Setcall gated to$RMM -eq 1so interactive runs do not blow up on the missing cmdletTest plan
\$RMM=1against a known-clean machine — expect Remote custom field set to 0\$RMM=1against a machine with CRD — expect Remote custom field set to 1\$RMMScriptPath\logs\chrome-remote-desktop-detect.logSummary by CodeRabbit
New Features
Documentation