Repository navigation
Write the UAT onboarding that does not exist yet - #2476
Conversation
Three places still describe the web dashboard as it behaved before the Custom Views composer landed, and all three are the ones an operator reads while deciding what to expose. DecideWebAuth exempts loopback from the CIDR check ONLY. In network mode a request from the box itself still has to carry a session cookie or a valid ?token= -- that changed when view create/update/delete and /api/compose/run made the surface writable, because a tokenless local process (a scheduled task, SSRF'd code, another user's session) could otherwise read the whole store and mutate views with no credential. The auth middleware is registered inside `if (networkMode)`, so a dashboard with no network block really is tokenless; the claim is only wrong for the exposed case, which is the case the sentence was about. "Hosts no write paths" is wrong for the same reason: POST/PUT/DELETE /api/views and POST /api/compose/run exist, and GET /api/session reports can_edit for any authenticated seat. That write IS the reason loopback authenticates, so the two corrections are one fact stated twice -- and the old text used the wrong half of it to justify the wrong conclusion. Also names the two responses a reader will actually see, because "an out-of-CIDR request is refused with 403" left the other arm unstated: an in-CIDR request with no credential gets the login form back as a 200, not a 401, and someone debugging by status code needs to know which is which. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
External UAT testers start shortly, and the only onboarding that exists is the root README's four-step Darling quick start, whose last step is "Open the viewer and point it at the store" with nothing about how. The web dashboard appears once, as a clause in a prose paragraph, with no procedure. Darling/README.md is 1,178 excellent lines of REFERENCE -- it answers "what does this setting do", which is a different question from "I have nothing, what do I type", and turning it into the second would cost it the first. So this adds the missing thing rather than growing the reference: one ordered procedure, docs/uat-onboarding.md, matching the shape postgres-first-target-runbook.md already established here -- a proof point at every step, quoting the log line, HTTP response or on-screen state that tells someone it worked without asking anyone. One file, not three. The three surfaces share most of a prerequisite: the install, the two .NET runtimes, the JSONC rule, --encrypt-password, the two-minute first start, the firewall verb. Split three ways that shared half gets written three times and goes stale in two of them. Each surface still stands alone -- Parts 2, 3 and 4 assume only Part 1 and never each other -- and the one fact that spans all three (which settings the file owns and which the store owns) is stated once, in Part 0, where both later parts can point at it instead of re-deriving it. Everything asserted was read out of the source rather than out of the docs, which is how the #1649 loopback drift fixed in the previous commit surfaced. The seven gotchas that cost real time get their own section with links back to where they bite, and Part 6 names the gaps -- no TLS on either endpoint, no verb that reprints a token, no runtime pre-check in the installer, no global read-only indicator -- because an admitted gap is cheaper than the support round that discovers it. README.md's step 4 becomes the two steps it was hiding (start the viewer, which needs no configuration on the service host; export a folder for a remote seat), plus a fifth for the two off-by-default surfaces, and both READMEs link into the new procedure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Restart-Service and --configure-firewall both need elevation, and the two LAN sections showed them in a bare code fence after prose that had moved on to talking about the config block. Every other elevated step in the document says so at the point of typing; these two did not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| What it *can* write is one thing: **Custom Views** — the saved dashboards and notebooks in | ||
| `config.custom_views`, created and edited through the built-in composer, plus `POST /api/compose/run` to | ||
| preview one. That is the `viewer` role's single narrow write grant, and it is available to **any authenticated | ||
| seat**, not only a local one. It is also the reason the loopback token exemption went away (see 3.5). |
There was a problem hiding this comment.
"That is the viewer role's single narrow write grant" is not accurate at the role level. DarlingManagedRoles.cs:494-496 and Darling/tools/provision-roles.sql:133-135 also grant viewer INSERT, UPDATE, DELETE on config.database_state_expected (the WPF Viewer's per-database state-override editor, #1986). It's true that grant isn't reachable through any web-dashboard HTTP route (no matching route in DarlingWebEndpoints.cs), so the claim about what the web dashboard can write still holds — but "the role's single narrow write grant" overstates it and would mislead someone auditing what the viewer role can actually do. Suggest scoping this to "the web dashboard's only write path" rather than describing it as the role's only grant.
| ``` | ||
|
|
||
| **What a web token can reach.** The web dashboard is **read-only over the collected store** — it connects as the least-privilege `viewer` role and hosts no write paths and no live-server queries (no `analyze_server`, no plan re-execution). A token-holder can view everything collected, change nothing, and reach no monitored server — which is why loopback stays tokenless here while MCP's does not. | ||
| **What a web token can reach.** The web dashboard is **read-only over the collected store** — it connects as the least-privilege `viewer` role and hosts no live-server queries (no `analyze_server`, no plan re-execution). Its one write path is the Custom Views composer, which is that role's one write grant: INSERT/UPDATE/DELETE on `config.custom_views` and nothing else. So a token-holder can read everything collected and author saved views, and reach no monitored server — but that write is why an exposed dashboard authenticates loopback too, exactly as MCP does. |
There was a problem hiding this comment.
"that role's one write grant: INSERT/UPDATE/DELETE on config.custom_views and nothing else" is factually wrong — the viewer role also has INSERT/UPDATE/DELETE on config.database_state_expected (DarlingManagedRoles.cs:494-496, Darling/tools/provision-roles.sql:133-135, the WPF Viewer's per-database state-override editor, #1986). That second grant isn't exposed by any web-dashboard HTTP route, so "the web dashboard's one write path" is accurate, but "that role's one write grant ... and nothing else" isn't. Same issue repeated in docs/uat-onboarding.md ("the viewer role's single narrow write grant"). Worth tightening the wording so it describes the dashboard's write surface, not the role's total grants.
The seat section listed "a `— read-only seat` window-title suffix" among the ways to tell which seat you have. No such suffix exists on the main window. What exists is one DIALOG title -- DatabaseStateOverridesWindow appends "(read-only seat — changes cannot be saved)" -- which a reader looking at the main title bar will not find. Sending a UAT tester to check a title bar that says nothing is worse than telling them there is nowhere central to look, so it now says the latter and points at Part 6, where the missing global indicator is already listed as a known gap. That gap is filed as #2479 item 3. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Reviewed. This is a docs-only PR (no T-SQL/C# changes), so the style guidelines in CONTRIBUTING.md don't directly apply; I focused on factual accuracy of the technical claims against the current source, broken links/anchors, and Lite/Darling parity. Verification approach: cross-checked ~20 specific claims (auth behavior in Result: everything checked out except one recurring inaccuracy, flagged inline in two places ( Everything else — the No Lite/Darling parity concern: this PR only touches Darling's onboarding path, which is the one that actually lacked a procedure (Lite's existing 3-step quick start already covers its single-surface flow). |
Review caught both new sentences claiming config.custom_views is the viewer role's only write grant. It is not: DarlingManagedRoles.cs and provision-roles.sql also grant INSERT/UPDATE/DELETE on config.database_state_expected, for the Viewer's per-database state-override editor (#1986). The claim I wanted was about the WEB HOST's reachable surface, which really is custom views alone -- no route in DarlingWebEndpoints and no reference in wwwroot touches database_state_expected. Saying that as a fact about the ROLE overstated the least-privilege boundary in the one direction a reader would act on, which is the whole reason the previous commit exists. Worth stating rather than just narrowing, because the second grant is currently spent by nobody: GetDatabaseStateExpectationsAsync gates its writes on IsReadOnly, and IsReadOnly is the has_table_privilege probe on config_mute_rules -- which the viewer role fails by design. So a viewer-role seat holds the grant and cannot reach the editor that uses it, on either surface. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Gotcha 11 said a screen of Warning statuses "is not an incident" because status is "collection freshness, not health". That is wrong, and wrong in the worst available direction for a monitoring tool: it tells a new user their first reflex on seeing amber should be to disregard it. ServerHealthBands.ClassifyBand returns Warning for three unrelated causes. A metric at warning severity -- CPU, blocking, deadlocks, memory, threads -- returns Warning straight out of the overallMetricSeverity switch, and that IS a real condition on the server. Awaiting-first-collection returns Warning before that switch is reached. Stale collection returns it after, through the hasCollectorErrors arm the viewer re-uses to carry staleness. The advice survives in narrowed form, because the original observation was not baseless: on a fresh install every server is awaiting first collection at once, and at fleet scale staleness is routine. So an opening screen of amber usually is not an incident. What it must not become is a rule for every screen after that one. It now names the three causes and points at the tooltip #2429 added, which is built from the same metric rows shown underneath the status word, so it cannot disagree with them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#2476's "What it will not do" listed the wait-type picker, tempdb, the perfmon picker, Running Jobs and the configuration grids as missing. All five ship on this branch, and a UAT tester reading a list of absences that are no longer absent is worse served than by no list at all. Two counts were also wrong and are corrected from the XAML: the viewer has nineteen top-level per-server tabs, not fourteen, and 65 TabItems counting the nested ones. What remains missing is now divided by WHY, because the three groups have different answers. Reads the service serves that no page shows (the eight PostgreSQL ones, blocked by the fleet payload carrying no target-engine discriminator; the nested-array scoped config; store-wide metrics). Viewer data with no read endpoint at all, which no web work reaches. And the desktop things a web imitation would be worse than — plan viewer, heatmap, block chain, deadlock graph, and the grids' own filter/export/repro/drill-down affordances. The exclusion bullets above it are untouched: no plan analysis, no analyze_server, no config_command enqueue, no alert tuning. Those are the viewer role's grants and the web host having no write path, and nothing here changes them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
External testers start shortly and there is no path for them. The root README's Darling quick start is four steps ending at "Open the viewer and point it at the store to browse the fleet" — which is the whole instruction for the WPF viewer. The web dashboard appears once, as a clause inside a prose paragraph, with no procedure at all. MCP has a Lite-shaped Setup section that names the wrong port and the wrong verb for Darling.
Darling/README.mdis not the answer, and it should not be made into the answer. Its 1,178 lines are a genuinely good reference: they tell you what a setting does, what a role can reach, why standbys are gated one way here and another way there. An onboarding answers a different question — I have nothing, what do I type — and rewriting the reference to answer it would cost the thing it is already good at.So this adds the missing artifact and links into the existing one rather than duplicating it.
The layout, and why
One file:
docs/uat-onboarding.md. It matchesdocs/postgres-first-target-runbook.md, which already established the shape here — an ordered procedure with a proof point at every step, deferring to the README for field-level detail.One file rather than one per surface, because the three surfaces share most of a prerequisite: the install location rule, the two .NET runtimes, the JSONC rule,
--encrypt-password, the two-minute first start,--configure-firewall. Split three ways, that shared half gets written three times and goes stale in two of them. Independent followability is kept structurally instead: Part 1 is explicitly the shared one, and Parts 2, 3 and 4 assume only Part 1 and never each other. The one fact that genuinely spans all three — which settings the file owns and which the store owns — is stated once in Part 0 so the later parts point at it rather than each re-deriving it.Every step ends with the log line, HTTP response, or on-screen state that proves it worked. Those are quoted from the source that emits them, not paraphrased.
README.mdstep 4 is replaced. It was the weakest sentence in the document. It becomes the two steps it was hiding — start the viewer, which needs no configuration at all on the service host because it finds the samedarling.jsonone directory up; and export a folder for a seat on another machine — plus a fifth step for the two off-by-default surfaces. Both READMEs link into the new procedure.The correctness fix in the first commit
Verifying the gotchas against source turned up a doc drift worth its own commit, because it is the kind that changes what someone chooses to expose.
Darling/README.mdanddarling.sample.jsonstill say the web dashboard's loopback stays tokenless while the endpoint is LAN-exposed.DecideWebAuthexempts loopback from the CIDR check only; in network mode a request from the box itself still needs a session cookie or a valid?token=. That changed when the Custom Views composer added view create/update/delete and/api/compose/runand the surface stopped being read-only. The same fact makes "hosts no write paths" wrong two paragraphs later — and the old text was using that wrong half to justify the wrong conclusion about loopback, so the two are one correction stated twice.A dashboard with no
networkblock genuinely is tokenless (the middleware is registered insideif (networkMode)), so the claim is wrong only for the exposed case, which is the case the sentence was about. The fix also names the response an operator will actually see on the other arm: an in-CIDR request with no credential gets the login form as a 200, not a 401.What was verified against source
Everything the doc asserts was read out of the code, not out of the docs. In particular: the store/file authority split for
mcp/web(ResolveEndpointToggle, and thenetworkblock held for the process lifetime); the firewall rule taking its port from the control plane, and the narrowed-but-not-closed window on an upgrade of a moved-port box;--encrypt-passwordwriting its prompt to stderr; DPAPILocalMachinescope on every blob; the read-only probe, its fail-safe arms, and the remote seat defaulting torole: "viewer"while the local seat defaults toconnectAs: "admin"; the~2 minutesfirst start and the log sequence that ends it.What a UAT user needs that the product does not do yet
Part 6 states these plainly rather than letting testers find them one support round at a time: no TLS on either HTTP endpoint; no verb that reprints an existing MCP or web token;
install-darling.ps1not checking for the .NET runtimes, so a missing one surfaces as a raw .NET host error; no global read-only indicator in the viewer; rejected 401/403/400 requests leaving nothing in the service log.