ge-aerospace/shopdb-flask
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Report mapped drives from both Windows sources, worse status wins

Browse Source 
Windows exposes mapped drives twice and the two views disagree. Captured on one
box at one moment, same two mappings: Get-SmbMapping reported OK for both while
net use reported Disconnected and Reconnecting. Collecting only Get-SmbMapping -
the nicer, more structured one - would have reported a healthy Q: on a machine
whose Q: was down, which is the exact failure this telemetry exists to catch.

So collect both and keep them apart. The probe emits netuse and smbmapping as
separate keys and does not reconcile them; the disagreement is the signal and
normalising it in the agent would discard it before anyone could see it.

The server merges by drive letter and shows the WORSE of the two. A drive that
either source calls broken is broken for the operator, and a drive neither
source gave a status for renders as Unknown rather than blank - an unannotated
row reads as healthy, so a failed check would present as a working drive.
Drives seen by only one source are kept, not dropped: Get-SmbMapping is absent
on some builds and session types, and requiring agreement would silently hide
every mapped drive on those machines.

No active reachability testing. Windows already knows the status, so touching
each share would be wasted work and would reintroduce the hang risk the bounded
probe exists to avoid.

The probe stays unelevated in the operator's session. UAC token separation
gives an elevated context a different mapped-drive table, often empty, so
elevating this later would quietly empty the card.

Adds a commented-out remap_drive seam in the command catalog. Not implemented,
but placed so the next person finds the user-session and elevation constraints
before writing it rather than after.

The merge logic has no test coverage yet; the docstring names the three
behaviours that must not be broken.
This commit is contained in:
benjamin.baker
2026-09-05 05:47:31 -04:00
parent 511ded1ba9
commit aa901c4111
4 changed files with 711 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

130
agent/ShopDbEndpointAgent.Core/UserContextProbe.cs
View File

@@ -2,8 +2,9 @@ using Microsoft.Extensions.Logging;
namespace ShopDbEndpointAgent.Core;
// Runs a short PowerShell probe as the interactive user and returns what it
// wrote to the spool.
// Collects the session-scoped data that does not exist in the agent's own
// context, by running a short PowerShell probe IN THE CALLING PROCESS'S
// SESSION and writing the result to the user's spool file.
//
// Collects only data that genuinely does not exist in the agent's own context:
// netuse - mapped drives are per logon session; "net use" from
@@ -15,15 +16,18 @@ namespace ShopDbEndpointAgent.Core;
// it. Sampled at probe time only, so treat it as a coarse
// "was anyone at the machine" signal, not live presence.
//
// This runs from `shopdbagent.exe --collect-usercontext`, which the tray - and
// only the tray - invokes, because the tray is already running as the signed-in
// operator. The service does NOT call this: it previously tried to descend into
// the session with CreateProcessAsUser, which is the direction that has to
// duplicate a token, is what endpoint protection watches for, and produced
// nothing on a live endpoint. Nothing here crosses a privilege boundary now;
// the probe simply runs as whoever the tray is.
//
// The script is embedded rather than shipped as a .ps1 so the probe cannot drift
// from the agent binary or be tampered with on disk between cycles.
public sealed class UserContextProbe
{
// The probe is bounded well below the telemetry cadence. If it cannot finish
// in this time the cycle simply reports no session data; it never delays the
// heartbeat.
private static readonly TimeSpan ProbeTimeout = TimeSpan.FromSeconds(45);
private const string ProbeScript = """
$ErrorActionPreference = 'Stop'
$result = [ordered]@{}
@@ -51,6 +55,30 @@ public sealed class UserContextProbe
}
$result['netuse'] = $netuse
# Second, independent view of the same drives. Win32_NetworkConnection
# is the "net use" table and Get-SmbMapping is the SMB client's own,
# and they disagree: a share can be Unavailable in one and absent or
# OK in the other. Neither is authoritative, so both are reported as
# collected and the server decides what to show. Normalising them
# here would discard exactly the disagreement that is diagnostic.
$smbmapping = @()
try {
Get-SmbMapping -ErrorAction Stop |
ForEach-Object {
$smbmapping += [ordered]@{
localpath = $_.LocalPath
remotepath = $_.RemotePath
status = [string]$_.Status
}
}
} catch {
# Absent on older builds and in some session types. A missing
# source is not a drive fault, so it is recorded and left out
# rather than reported as a failed mapping.
$result['smbmappingerror'] = $_.Exception.Message
}
$result['smbmapping'] = $smbmapping
$listeners = @()
try {
$ownprocesses = @{}
@@ -113,74 +141,72 @@ public sealed class UserContextProbe
exit 0
""";
private readonly UserContextSpool spool;
// How long the probe is allowed to take. Generous: this no longer runs on
// the telemetry path, so an unresponsive WMI call delays nothing but itself.
private static readonly TimeSpan ProbeTimeout = TimeSpan.FromSeconds(60);
private readonly ILogger<UserContextProbe> logger;
public UserContextProbe(UserContextSpool spool, ILogger<UserContextProbe> logger)
public UserContextProbe(ILogger<UserContextProbe> logger)
{
this.spool = spool;
this.logger = logger;
}
// Returns the probe payload, or null when there is no interactive session or
// the probe produced nothing usable. Never throws: session-scoped data is a
// nice-to-have and must not be able to fail a telemetry cycle.
public Dictionary<string, object?>? TryCollect()
// Run the probe in THIS process's session and write the spool file.
// Returns true when the probe wrote a result.
//
// Called from the --collect-usercontext entry point, which the tray runs as
// the signed-in operator. Never throws: this is background collection and a
// failure must not take the tray or the agent process down with it.
public bool Collect()
{
string? scriptPath = null;
try
{
var resultPath = spool.TryGetResultPath(out var reason);
if (resultPath is null)
{
logger.LogDebug(
"Skipping user-context collection: {Reason}", reason);
return null;
}
spool.EnsureSpoolDirectory(resultPath);
spool.ClearStaleResult(resultPath);
scriptPath = Path.Combine(
Path.GetTempPath(), $"shopdb-usercontext-{Guid.NewGuid():N}.ps1");
File.WriteAllText(scriptPath, ProbeScript);
var arguments =
"-NoProfile -NonInteractive -ExecutionPolicy Bypass -WindowStyle Hidden -File "
+ "\"" + scriptPath + "\"";
var outcome = InteractiveProcessLauncher.LaunchInActiveSession(
"powershell.exe", arguments, Path.GetTempPath(), waitForExit: true,
waitTimeout: ProbeTimeout);
if (!outcome.Launched)
var startInfo = new System.Diagnostics.ProcessStartInfo
{
logger.LogDebug(
"The user-context probe did not launch: {Error}", outcome.Error);
return null;
FileName = "powershell.exe",
Arguments =
"-NoProfile -NonInteractive -ExecutionPolicy Bypass -WindowStyle Hidden -File "
+ "\"" + scriptPath + "\"",
WorkingDirectory = Path.GetTempPath(),
UseShellExecute = false,
CreateNoWindow = true,
};
using var process = System.Diagnostics.Process.Start(startInfo);
if (process is null)
{
logger.LogWarning("The user-context probe could not be started.");
return false;
}
if (outcome.TimedOut)
if (!process.WaitForExit((int)ProbeTimeout.TotalMilliseconds))
{
logger.LogWarning(
"The user-context probe did not finish within {Seconds} seconds.",
ProbeTimeout.TotalSeconds);
return null;
TryKill(process);
return false;
}
if (outcome.ExitCode is int exit && exit != 0)
if (process.ExitCode != 0)
{
logger.LogWarning(
"The user-context probe exited with code {ExitCode}.", exit);
return null;
"The user-context probe exited with code {ExitCode}.", process.ExitCode);
return false;
}
return spool.TryConsumeResult(resultPath);
return true;
}
catch (Exception exception)
{
logger.LogWarning(exception, "The user-context probe failed.");
return null;
return false;
}
finally
{
@@ -193,10 +219,22 @@ public sealed class UserContextProbe
}
catch
{
// A leftover temp script is harmless; it is overwritten by
// name on the next cycle and cleaned by the OS.
// A leftover temp script is harmless; it is uniquely named
// and cleaned by the OS.
}
}
}
}
private void TryKill(System.Diagnostics.Process process)
{
try
{
process.Kill(entireProcessTree: true);
}
catch (Exception exception)
{
logger.LogDebug(exception, "Could not stop the timed-out user-context probe.");
}
}
}

482
docs/notes/endpoint-telemetry-and-ui-phases.md Normal file
View File

@@ -0,0 +1,482 @@
# Endpoint telemetry and PC Details UI - phased working notes
Scratch/reference notes for a multi-phase effort. Not a contract document. When
a phase lands, its details belong in AGENT-CONTRACT.md, the csproj changelog, or
the panel declarations - not here. Delete this file when all phases are done.
Branch: wilm. Current versions at time of writing: agent 1.8.0, tray 1.1.0.
---
## Ground truth established before scoping
Verified in code, not assumed. These constrain everything below.
**Panel groups are a hardcoded frontend list.**
`frontend/src/components/PluginAssetPanels.vue:423`:
const RENDERED_GROUPS = ['details', 'monitors', 'software', 'activity']
Any panel whose `group` is not in that list silently falls back to `details`.
Nothing is logged. The comment directly above it records that this has already
caused two disappearing-card incidents (`actions` -> `monitors`, and the
`metrics` -> `monitors` rename). Tab display labels are separately hardcoded in
`plugins/computers/frontend/views/PCDetail.vue:441-449`.
Consequence: introducing a new tab is a THREE-file change that must land in one
commit - panel `group` in `plugins/endpoints/plugin.py`, `RENDERED_GROUPS`, and
the PCDetail label map. Split it and cards vanish with no error.
**There is no pinned/locked card concept today.**
Layout is a CSS grid ordered purely by the integer `position` on each panel.
There is no "top row", no span control except one special case: the Network
interfaces table sets full-width span because at half width it wrapped every
cell to three lines (`PluginAssetPanels.vue:693`).
Consequence: "lock this card top-left/top-right" does not exist yet and is
itself a feature, not a config value. See Phase 5.
**Two of the requested cards are not plugin panels at all.**
Both the Entra/Intune card (`PCDetail.vue:202-243`) and the Network/IP card
(`PCDetail.vue:249-265`) are hand-written Vue inside PCDetail's details grid,
not declarations in `plugin.py`. Moving them is a component refactor, not a
`position`/`group` edit.
**There is no LAPS retrieval feature in this codebase.**
Searched: no LAPS button, endpoint, or credential path exists. The only hit is
a historical incident note in `plugins/tools/frontend/views/SecureSendCreate.vue:254`
about LAPS passwords once being sent over a chat link. The request assumes a
click-to-retrieve LAPS button already exists on the Entra card; it does not.
This needs its own scoping conversation - it is a credential-disclosure path
with real audit/permission implications and must not be smuggled in as part of
a layout change. See Open Questions.
**Existing panel positions in `monitors`** (so new cards do not collide):
10 status, 12 tracked services, 13 tracked processes, 14 hardware, 15 osinfo,
16 interfaces, 17 storage, 18 operator reports, 19 listeners, 40 user context.
`activity` uses 20. `software` uses 30.
---
## Phase 1 - Mapped drive reachability (agent + tray)
STATUS: code complete, NOT fully validated. See "Where we left off" at the end
of this file before resuming.
Highest value, and the reason the user-context work exists. Extends the probe
already added in agent 1.8.0 / tray 1.1.0.
**No active reachability testing.** An earlier draft proposed touching each
share to see if it responded, with a per-mapping timeout. Rejected: Windows
already reports the status, so probing the share is wasted work and reintroduces
the hang risk the bounded probe exists to avoid. Read the status, do not test it.
**The two status sources disagree, and that disagreement is the signal.**
Captured live on the same box at the same moment, same two mappings:
Get-SmbMapping net use
OK Q: ...specdata Disconnected Q: ...specdata
OK S: ...shared Reconnecting S: ...shared
`Get-SmbMapping` reported OK for both while `net use` reported Disconnected and
Reconnecting. So neither source alone is trustworthy: collecting only
Get-SmbMapping (the nicer, more structured one) would have reported a healthy
Q: on a box whose Q: was actually down - exactly the failure this card exists
to catch.
Collect BOTH and keep them as separate fields per mapping. Do not silently
reconcile them in the agent; the server/UI decides presentation, and preserving
the disagreement is diagnostically useful in its own right. The card should
surface the WORSE of the two rather than the prettier one.
Status vocabulary must include `Reconnecting` - it appears in the real capture
above and an earlier draft's `Ok`/`Disconnected`/`Unknown` set would have
forced it into the wrong bucket. Keep `Unknown` for probe failure so a failed
check never renders as green Ok.
**Elevation matters and confirms the tray approach.** The capture was taken
from an elevated shell (`start powershell -verb runas`). Because of UAC token
separation, an elevated context sees a DIFFERENT mapped-drive table than the
normal user - often empty. The tray runs unelevated as the operator, which is
the correct context to enumerate from. Do not "improve" this later by elevating
the probe; that would silently empty the card.
Mechanism: the probe already writes JSON to the spool file, so this rides the
existing handoff - run both commands in-session, write the combined result,
service reads it. No new transport.
Server: persist as part of the existing `usercontext` payload key (no new
column, no migration needed - it rides `usercontextjson`).
Note in code, commented out, per request: a future one-click remediation
command will re-establish a Disconnected mapping. Do not implement now, but
leave the command-catalog seam obvious. It will be an allowlisted action in
`plugins/endpoints/commands.py` and must run in the USER session (a SYSTEM
remap creates a mapping the operator cannot see, and per the elevation note
above an elevated remap has the same problem).
## Phase 2 - Cheap system-side telemetry (agent service)
All service-side, no session needed. Group these because they share the
inventory-snapshot plumbing and one migration.
- **Time sync offset and source** - collect; NOT surfaced in the UI. Wire for
alerting only. Kerberos fails past skew, so this is a high-value silent
predictor.
- **Disk errors / SMART predictive failure** - collect and surface on the
existing Storage card (Phase 4).
- **NIC link speed and duplex** - collect, pair into the existing Network
interfaces collection, NOT surfaced yet.
- **Boot type** - fast startup vs true cold boot. Explains why uptime lies and
why a "reboot" did not clear a fault. Collect now; UI placement undecided,
candidate for the Operating system or Hardware card.
- **Windows Update history, last 5** - surfaced in Monitors (Phase 4) for
pinpointing update-induced breakage. Updates are corporate-managed; this is
diagnostic only, not control.
- **Defender/AV health** - collect and persist, do NOT surface. Known to
produce false positives in this environment; stored for future wiring.
Explicitly dropped after discussion, do not collect:
certificates (corporate-managed), BitLocker (network join already requires it),
display topology (single monitor on the floor, trained users), session
type/RDP (RDP-in not permitted on the shop floor), logon server (local users).
**Pending file-rename operations** - CONFIRMED, collect. A stronger
reboot-required signal than the current `pendingreboot` key, and cheap.
Schema: these add columns to `endpointstates`. One migration, `0014`, with
idempotent guards and a real downgrade, shipped in the SAME change as the model
edit (a model referencing an unapplied column 500s every query on the table).
## Phase 3 - Service/Process Monitors tab split
Tab churn is exactly what has silently eaten cards twice before, so this phase
is deliberately isolated from any collection work.
- Rename the existing `Monitors` tab to `System Monitors`.
- Add a new `Service/Process Monitors` tab.
- Move `Tracked Services` (position 12) and `Tracked Processes` (13) into it.
Must land atomically across all three places:
1. `group` on those panels in `plugins/endpoints/plugin.py`
2. `RENDERED_GROUPS` in `PluginAssetPanels.vue`
3. the tab label map in `PCDetail.vue`
Also update the hardcoded panel-count assertion in
`tests/test_plugins/test_endpoints_plugin.py`
(`test_endpoints_asset_panels_are_computer_scoped`) - it has already needed
bumping once (7 -> 12) and will fail again here.
Printers: a location-based printer auto-detect collection is in progress
elsewhere. Reserve space and a position band for a printer card in the monitors
family so this work does not have to be redone when it lands. Do not build the
card now.
## Phase 4 - Card content additions
Depends on Phase 2 data existing.
- **Mapped drives card** - `Get-SmbMapping`-shaped. `Disconnected` in red,
`Ok` in green. Reuse existing badge/severity styling rather than inventing
new CSS. Room reserved for the future one-click fix action.
- **Storage card** - add disk error / SMART predictive failure to the existing
card rather than a new one.
- **Windows Update card** - last 5 updates, in the System Monitors tab.
- Boot type placement to be decided when the data is visible.
## Phase 5 - Header right-hand block (DEFERRED - revisit before building)
Status: on hold at user request. The original ask assumed pinned cards were a
config value; they are not, and building a pinning system was not what the user
wanted to sign up for. Not scheduled. Nothing below is committed.
The cascade worry that motivated pinning is explicitly parked - do not build
anything for it unless reflow problems actually surface.
**Revised direction: use the empty right-hand half of the existing header.**
The PC Details header is a full-width card whose right side is currently dead
space - the hostname, badges, PC type, OS and location all sit left, and the
Status / Remote access / Refresh / Restart row runs along the bottom. The block
to the right of the identity lines is empty at every width.
This is a better target than either pinned grid cells or extending the header
downward:
- It consumes space that already exists, so it costs ZERO additional vertical
height and cannot push the tab strip toward the fold. That was the hard
constraint, and this approach satisfies it structurally rather than by
budgeting.
- It sidesteps the Network interfaces width problem entirely (that table is
full-width by necessity and would fight a half-width pinned slot).
- Header content lives outside the panel grid, so it cannot be reflowed by
cards appearing or disappearing below it.
Candidates for the block, unresolved:
- Entra/Intune deep links (currently a Details-tab card). Strongest candidate:
they are navigation, not data, and the user has called out their outbound
usability value.
- IP / network summary (currently the hand-written Network card).
Density rules if this proceeds: inline label/value pairs, not nested cards. Do
not let the right block grow taller than the left identity block, or it starts
adding height after all and the zero-cost property is lost.
Still true and still unbuilt if this proceeds:
- `Collectors` is not a current tab or group. If IP History moves to a new tab,
that is the same three-file atomic change as Phase 3.
- Entra empty state, requested: show `Please provide DeviceID` /
`Please provide ObjectID` when the backend has no value, rather than an
empty or broken card.
LAPS is NOT a feature request - clarified by the user as an illustration of why
these outbound portal links matter to usability. No LAPS work is scoped.
## Phase 6 - Intune/Entra link bug (independent, can be done anytime)
Reported: the INTUNE and ENTRA buttons sometimes do not appear until the page
is refreshed. Diagnosed, root cause found, NOT yet fixed.
**Cause: a cold-cache race, not a render problem.**
`frontend/src/utils/siteSettings.js:9` caches only the RESOLVED settings object:
let settingsCache = null
async function loadSettings() {
if (settingsCache) return settingsCache
...
The in-flight promise is not cached, so every caller during the first load
fires its own `settingsApi.list()`.
In `plugins/computers/frontend/views/PCDetail.vue:513`, `getEntraDeviceUrls()`
is deliberately fire-and-forget (`.then(...)`, not awaited) so a settings
failure cannot block the page. Meanwhile `computersApi.get()` on line 519 IS
awaited. On a cold cache the settings response can therefore land AFTER the
computer data has rendered. Until it does, `entraUrlTemplate` /
`intuneUrlTemplate` are still `''`, so `fillTemplate()` returns null
(line 426: `if (!template || !value) return null`), and the `v-if` on the
anchors hides both links. A later refresh hits the warm cache and they appear.
That also explains the intermittency: it depends on which response wins, so it
varies by network timing and by whether another view already warmed the cache.
**Fix: cache the in-flight promise, not just the result.** Assign the promise
to `settingsCache` immediately so concurrent callers await the same request.
This is a one-line-shaped change in `siteSettings.js` and fixes every consumer
of site settings, not just this card - the same race exists for any view that
reads settings during mount.
Watch for: the existing `catch` sets `settingsCache = {}` on failure, which
permanently caches the empty result for the page session. Confirm whether a
failed settings load should stay cached (currently it does) or be retryable.
Do not change that behavior silently while fixing the race.
Verify after fixing that the links appear on a hard reload with a cold cache
and throttled network, which is the condition that reproduces it.
DONE. Fixed in `siteSettings.js`: the in-flight promise is now cached alongside
the result, and a failed load is cached for 30s rather than the page session.
Covered by `frontend/src/utils/siteSettings.spec.js` (6 tests). Open question 5
resolved in favour of the short TTL.
Measured side benefit: the old code assigned `settingsCache` only AFTER the
response resolved, so each sequential `await` re-entered `loadSettings()` before
the cache existed. `applyBranding()` awaits five settings in a row at bootstrap,
`getServicenowUrls()` four, `getEntraDeviceUrls()` two - all of which were
separate GETs on a cold load. Now one.
## Phase 7 - PCForm "Failed to load data" (NOT the same bug)
Separate from Phase 6 and NOT caused by it - `PCForm.vue` does not import
`siteSettings.js`. Surfaced on the F3KKSGH4 edit page.
Partially addressed, root cause still UNKNOWN.
Evidence: the form renders with reference data present (printer list populated,
dropdowns built) but record fields empty. That means the eight-way
`Promise.all` of reference endpoints SUCCEEDED and the failure is in the
edit-only block, `PCForm.vue` lines 589-635. The only unprotected call there is
`computersApi.get(route.params.id)`; `ensureModelLoaded()` already swallows its
own errors, so it is not the source.
Two candidates remain, with different fixes and different layers:
- the `computersApi.get` request fails (backend/route problem), or
- a TypeError while mapping the response into `form.value` (frontend shape
mismatch, plausibly from recent schema work).
Both land in the same catch and both printed the same generic string, which is
why this could not be diagnosed from the UI.
Done so far: the catch now uses `apiError(err, ...)` so the real server message
surfaces instead of a generic one, and the stale comment claiming a call
"handles its own failure" was removed - no such call remains in that list.
NEXT: reload the edit page and read the now-specific error. An axios status
code points at the backend; a TypeError names the property and points at the
mapping block. Do not guess between them.
Also worth doing regardless: a single failed reference list currently blanks
the whole form via `Promise.all` rejection. Degrading one dropdown instead
would be the behaviour the removed comment falsely claimed.
---
## Open questions - answer before the relevant phase
1. ~~LAPS~~ - RESOLVED: not a feature request. The user was illustrating why the
outbound Intune/Entra links matter to usability. No LAPS work scoped. The
security concerns noted earlier stand if it is ever revisited.
2. ~~Pending file-rename operations~~ - RESOLVED: collect. Folded into Phase 2.
3. ~~Phase 5 pinning / cascade symptom~~ - RESOLVED: not worrying about the
cascade unless it surfaces. Pinned cards dropped in favour of the header's
empty right-hand block; Phase 5 deferred.
4. **`Collectors` tab** - only relevant if Phase 5 proceeds. New tab for one
moved card, or does IP History belong in an existing tab?
5. **Failed settings load caching** - Phase 6. Should a failed
`settingsApi.list()` stay cached for the page session (current behavior) or
be retryable? Decide while fixing the race, do not change it silently.
## Sequencing
1 -> 2 -> 3 -> 4. Phase 6 is independent and can be slotted in anytime; it is
small, self-contained, and fixes a user-visible bug, so it is a good candidate
to do early. Phase 5 is deferred and unscheduled.
Phases 1 and 2 are additive and independently deployable. Phase 3 is a
contained tab move. Phase 4 needs 2.
Agent/tray version bumps: Phase 1 touches the probe (tray behavior) and Phase 2
touches the service collector. Both need csproj changelog entries. Note the
existing coupling hazard - `Program.cs` has no unknown-arg guard, so an older
agent receiving a newer tray's verb starts a full second host rather than
no-oping. Fix that guard before any deployment where tray and agent could skew.
## Phase 8 - fetchAllPages returns a bare array (LAST, and it affects the above)
Deliberately last: it is a cross-cutting API-surface change, and every earlier
phase should land against the current shape rather than a moving one.
`frontend/src/api/index.js:131` - `fetchAllPages()` returns the ROWS:
return items
Every other method on every API object returns the axios RESPONSE. So callers
must remember, per call site, which of two shapes they are holding:
const rows = await vendorsApi.listAll() // array
const response = await vendorsApi.list() // { data: { data: [...] } }
Reading `.data.data` off the array yields `Cannot read properties of undefined
(reading 'data')`. Inside a `Promise.all` that rejects the whole batch, so one
wrong unwrap blanks an entire form - and if it throws before the record fetch,
NO request is made at all, which makes it look like a backend or schema fault.
That is exactly how the PC edit page presented, and why it was misdiagnosed
twice before the log showed `/api/computers/729` was never requested.
Known history - this class of bug has now hit at least four times:
- `MachineForm.vue` `pcsRes` (fixed previously, comment left in place)
- `PCForm.vue` `vendorRes` (fixed now)
- `MachineForm.vue` `vendorRes` and `buRes` (fixed now - the warning comment was
already sitting three lines away and the adjacent defects were still missed)
- `MeasuringToolForm.vue` `buResponse` (fixed now)
`ConfigurationPoliciesList.vue:187` defends with
`Array.isArray(value) ? value : ...`, which is evidence the shape is not
trustworthy at the call site.
Why this matters to the phases above: Phase 1/2 UI work adds call sites that
consume paged reference data, and Phase 5 (if revived) moves cards between
components. Any new `listAll()` caller is another chance to hit this. Landing
the phases first and this last means one migration pass, not a moving target.
Options, in order of preference:
1. Make `fetchAllPages` return `{ data: { data: items } }` so every API method
has ONE shape. Mechanical but touches every existing `listAll()` caller in
the same commit, or they break in the opposite direction. Highest value,
highest blast radius - needs the full grep and a single atomic change.
2. Rename to something the shape is obvious from (`fetchAllRows`,
`listAllRows`). Cheap, no behavior change, keeps the trap but makes it
visible at the call site. Good interim step if 1 is too large right now.
3. Leave it and rely on review. Rejected - four occurrences say review does not
catch it, and one of them happened directly beneath a comment warning about
it.
Whichever is chosen, do the full-codebase grep for `listAll(` first; there are
callers in core views, print views, and at least seven plugin frontends.
---
# Where we left off
Written mid-session when the test tooling stopped working. Read this first.
## Phase 1 is code complete but NOT fully validated
What landed, all uncommitted in the working tree:
**`agent/ShopDbEndpointAgent.Core/UserContextProbe.cs`** - added `Get-SmbMapping`
alongside the existing `Win32_NetworkConnection` capture, emitted as a separate
`smbmapping` key with its own `smbmappingerror`. The two sources are NOT
reconciled in the probe, deliberately - see the Phase 1 notes above. Still
unelevated, still in the operator session.
**`plugins/endpoints/api/routes.py`** - added `_drive_letter`,
`_DRIVE_STATUS_RANK`, `_worse_drive_status`, `_mapped_drives`, and pointed
`panel_usercontext()` at `_mapped_drives()`. Merges the two views by drive
letter and shows the WORSE status. Carries a long WARNING comment naming the
three load-bearing behaviours; that comment is doing real work, leave it.
**`plugins/endpoints/commands.py`** - commented-out `remap_drive` seam with the
user-session/elevation reasoning. Comment only, no behavior.
## Validation state - the important part
Last KNOWN-GOOD full run: `8 failed, 2489 passed, 11 skipped`. All 8 failures
are pre-existing and unrelated:
- 3x `test_endpoints_plugin.py` timestamp tests - these fail ONLY on Windows.
`_in_zone()` calls `time.tzset()`, which does not exist on Windows (Unix-only
in CPython), so it raises AttributeError before the assertions ever run.
`_parse_dt` itself is correct. These pass on CI. Consider a
`skipif(not hasattr(time, 'tzset'))` so the suite is honest on Windows.
- 5x docs / security-headers / plugin-guard, all pre-existing.
TWO CHANGES LANDED AFTER THAT RUN AND ARE UNVERIFIED:
1. `_mapped_drives()` now renders `(Unknown)` for a drive neither source gave a
status for. Previously it rendered blank, which reads as healthy - the exact
silent-green failure the warning comment warns about. Small but real,
behavioral, and untested.
2. The `commands.py` seam. Comment only, zero risk.
`get_errors` on both changed files is clean, but that only proves they parse.
## Tooling failure that stopped validation
Every `run_command_in_terminal` call began failing with "A task was canceled",
including a two-file pytest run that should finish in under a second, and a
background launch that never wrote its `Tee-Object` output file. Two full-suite
runs had succeeded earlier in the same session, so this is not slowness - the
execution path went down partway through.
Test Explorer discovers all 2508 tests but returns zero results for every
filter tried (`TypeName`, `MethodName`, `FullyQualifiedName`). Likely needs a VS
restart.
## Next actions, in order
1. Restart VS, then run `python -m pytest tests/test_plugins/test_endpoints_plugin.py -q`.
Expect 3 failed (the tzset ones) / 23 passed. Anything else is ours.
2. Consider adding tests for `_mapped_drives()` - currently zero coverage on
logic that decides whether a broken drive renders as broken. The cases that
matter: sources disagree (worse wins), drive in only one source (kept), no
status at all (Unknown, not blank), `Get-SmbMapping` absent entirely
(degrades to netuse alone).
3. Only then start Phase 2. Do not begin new collection work on an unverified
base - the untested `(Unknown)` delta would get buried under it.
## Deferred, agreed explicitly
Agent and tray republish happens at the END of all phases, not per-phase. The
server change is safe to deploy ahead of the agent: until the new probe ships,
`smbmapping` is simply absent from the payload and `_mapped_drives()` falls
through to `netuse` alone, rendering exactly as it does today. No ordering
constraint between them.

133
plugins/endpoints/api/routes.py
View File

@@ -672,6 +672,125 @@ def panel_listeners(asset_id: int):
return success_response(rows)
def _drive_letter(value):
"""Normalise a drive to its bare letter so the two sources can be paired.
net use reports 'Z:' and Get-SmbMapping reports 'Z:' or occasionally a
trailing separator, so compare on the letter alone.
"""
text = (value or '').strip().rstrip('\\/')
return text.upper() or None
# Worst-first. The panel shows the worse of the two sources for a drive, so a
# share that one source calls OK and the other calls Unavailable is reported as
# Unavailable: a drive that is broken in either view is broken for the operator.
_DRIVE_STATUS_RANK = {
'unavailable': 0,
'disconnected': 0,
'paused': 1,
'reconnecting': 2,
'ok': 3,
'connected': 3,
}
def _worse_drive_status(left, right):
"""Return whichever status is worse, preferring a known one over None.
An unrecognised status ranks as Reconnecting, deliberately below OK: a
status we cannot interpret must not be assumed healthy.
"""
candidates = [s for s in (left, right) if s]
if not candidates:
return None
unknown_rank = _DRIVE_STATUS_RANK['reconnecting']
return min(
candidates,
key=lambda s: _DRIVE_STATUS_RANK.get(str(s).strip().lower(), unknown_rank)
)
def _mapped_drives(context):
"""Merge the two mapped-drive views into one row per drive letter.
Windows exposes mapped drives through Win32_NetworkConnection (the 'net use'
table) and through Get-SmbMapping, and they do not always agree - a share can
be Unavailable in one and OK or absent in the other. The probe collects both
without reconciling them, so the disagreement survives to here.
A drive present in only one source is still a drive: it is reported with the
status that source gave it, rather than dropped for lack of a second opinion.
WARNING - this function is not covered by tests. Three behaviours here are
load-bearing and easy to break while "simplifying":
1. Worse-status-wins. Never report the healthier of the two sources. A
drive that either source calls Unavailable IS unavailable to the
operator, and rendering it OK sends someone to a share that is down.
2. Single-source drives are kept, not dropped. Get-SmbMapping is absent on
some builds and session types; requiring agreement would silently hide
every mapped drive on those machines.
3. Unknown statuses rank as Reconnecting, not as OK. An unrecognised
string must never be optimistically treated as healthy.
Changing any of these makes a broken drive look fine, which is a silent
failure - the panel stays green and nobody is told. If you touch this,
add the tests first.
"""
drives = {}
netuse = context.get('netuse')
if isinstance(netuse, list):
for entry in netuse:
if not isinstance(entry, dict):
continue
letter = _drive_letter(entry.get('localname') or entry.get('local'))
if not letter:
continue
drives[letter] = {
'remote': entry.get('remotename') or entry.get('remote') or '',
'status': entry.get('status'),
}
smbmapping = context.get('smbmapping')
if isinstance(smbmapping, list):
for entry in smbmapping:
if not isinstance(entry, dict):
continue
letter = _drive_letter(entry.get('localpath'))
if not letter:
continue
existing = drives.get(letter)
if existing:
existing['status'] = _worse_drive_status(
existing.get('status'), entry.get('status'))
existing['remote'] = existing['remote'] or entry.get('remotepath') or ''
else:
drives[letter] = {
'remote': entry.get('remotepath') or '',
'status': entry.get('status'),
}
rows = []
for letter in sorted(drives):
drive = drives[letter]
status = (drive.get('status') or '').strip()
remote = (drive.get('remote') or '').strip()
label = '{} {}'.format(letter, remote).strip()
# A drive neither source gave a status for is Unknown, never blank. An
# unannotated row reads as healthy, so a failed check would render as a
# working drive - the one outcome this card must never produce.
if not status:
status = 'Unknown'
# Only the problem states are annotated. Tagging every healthy drive
# with "(OK)" would bury the one that is not.
if status.lower() not in ('ok', 'connected'):
label = '{} ({})'.format(label, status)
rows.append(label)
return rows
@endpoints_bp.route('/asset/<int:asset_id>/usercontext', methods=['GET'])
@require_permission('endpoints.view')
def panel_usercontext(asset_id: int):
@@ -681,6 +800,9 @@ def panel_usercontext(asset_id: int):
session 0, so they are collected by a probe run in the logged-in user's
session. Nothing is reported when no one is signed in, which is a valid
state and not a failure.
Mapped drives arrive from two sources that can disagree; the worse of the
two statuses is shown so a half-broken share is never rendered as healthy.
"""
state = EndpointState.query.filter_by(assetid=asset_id).first()
if not state:
@@ -690,21 +812,14 @@ def panel_usercontext(asset_id: int):
if not isinstance(context, dict) or not context:
return success_response({'fields': []})
netuse = context.get('netuse')
drives = []
if isinstance(netuse, list):
for entry in netuse:
if isinstance(entry, dict):
local = entry.get('localname') or entry.get('local') or ''
remote = entry.get('remotename') or entry.get('remote') or ''
drives.append('{} {}'.format(local, remote).strip())
drives = _mapped_drives(context)
listeners = context.get('userlisteners')
listenercount = len(listeners) if isinstance(listeners, list) else None
fields = [
{'label': 'Probed user', 'value': context.get('username')},
{'label': 'Collected', 'value': context.get('timestamp')},
{'label': 'Collected', 'value': context.get('collectedat')},
{'label': 'Idle', 'value': _format_uptime(context.get('idleseconds'))},
{'label': 'Mapped drives',
'value': ', '.join(drives) if drives else None, 'mono': True},

21
plugins/endpoints/commands.py
View File

@@ -102,6 +102,27 @@ COMMAND_ALLOWLIST = {
}
# Reserved seam - one-click mapped-drive remediation. NOT implemented; this is a
# placement note so the next person adds it here rather than inventing a new
# transport. When it lands it will re-establish a Disconnected mapping surfaced
# by the user-context card:
#
# 'remap_drive': {
# 'label': 'Reconnect mapped drive',
# 'description': 'Re-establish a disconnected network drive mapping.',
# 'targetkind': 'drive',
# },
#
# It MUST execute in the interactive USER session. A SYSTEM-context remap creates
# a mapping in SYSTEM's own drive table that the operator cannot see, so the card
# would keep reporting the drive as broken while the "fix" silently succeeded
# somewhere nobody is looking. An ELEVATED remap fails the same way, because UAC
# token separation gives the elevated context a different drive table - the same
# reason the probe itself runs unelevated from the tray.
#
# CommandExecutor.RunScriptInUserSession is the existing path for this.
# Actions that demand a permission stricter than the base endpoints.command.
# The enqueue route requires the caller to ALSO hold the mapped permission, so a
# destructive action (a remote reboot) is a heavier grant than a routine toggle.