MCP and web host defaults: form sign-in, token length floor, sign-in wait, write-tool token gate

[erikdarlingdata]

What changes

Web dashboard

• A token is accepted only from the sign-in form (POST /auth/login). A token in the address (?token=) signs no one in and is never compared, whatever its value. Without a session cookie, the request gets the sign-in page (an /api/ path gets 401), and the service log line says a token in the URL is not accepted and that scripts sign in with a form POST to /auth/login and then present the session cookie. The dashboard takes no bearer header (the MCP endpoint does).
• The exchange of an address token for a cookie, and the redirect that stripped it, are removed. WebAuthAction and WebRequestAction no longer have SetCookieAndRedirect, DecideWebAuth and DecideWebRequest take no token verdict, and BuildPathWithoutToken is gone. A token is never an input to the decision functions; the form's POST is handled as a flow route before them.
• A script signs in the way the form does: it posts the token to /auth/login and then presents the session cookie it gets back (curl -c jar -d "token=<token>" http://<host>:5153/auth/login, then curl -b jar http://<host>:5153/api/fleet).
• A wrong token keeps the page the form was covering. The sign-in form served as the answer to a wrong token carries that page in its hidden return field (the address bar then names /auth/login, not the page), so a right token on the next try lands where a first-try success does. The landing is SignInLandingPath: a local path only, and a target that is a sign-in or sign-out path lands on /. The page script fills the field from the address only when the address is not /auth/login, and the SSO link takes the same target.
• Failed sign-ins from one address wait longer each time (3 free failures, then 2 s doubling up to 5 min). Each sign-in is checked against the wait and counted in one step, after its body has been read and before its token is compared, so attempts that arrive together, or that are held open and released together, are counted one by one and only the allowance (the 3 free ones plus the one that starts the wait) is compared. During the wait no token is compared: a request gets 429 with Retry-After, a correct token included. The wait is kept in memory, so a service restart clears it; the 429 page and the README say so, and an operator held out by a wait can restart the service.
• A success gives back only its own attempt. Other clients behind the same address keep their count.
• A sign-in that is held up only by sign-ins from the same address that are still being checked waits for them (at most 5 seconds, and it ends if the request is dropped) instead of getting 429. If they match, their reservations are given back and it goes through. If they do not match, their misses are recorded, the wait starts and it gets 429 without a comparison. If the 5 seconds pass with them still unresolved, it gets 429 with Retry-After equal to the delay the source's recorded misses plus those still being checked earn (DelayAfter(Misses + InFlight)), which is the real hold. At most 4 wrong tokens (the 3 free ones plus the one that starts the wait) are compared per address before its wait starts, however many requests arrive at once.
• IPv6 sources are grouped by their /64, every loopback address (127.0.0.0/8, ::1, IPv4-mapped loopback) is one source, and a total budget of 60 attempts per hour holds across all sources that have not signed in during the last hour, so changing addresses does not multiply the attempts. An address that signed in during the last hour is not held by the total budget; its own wait still applies. The hour matches the window the total is counted over, a success renews it, and at most 256 such addresses are remembered. All these numbers are constants in DarlingSignInThrottle, each with the reason beside it; there is no setting.
• When the total budget holds, the 429 page and the refusal line say so: the wrong tokens came from other addresses, a service restart clears the hold, and the failing client can be fixed or taken out of allowFrom.
• The sign-in POST must come from the dashboard's own page. Its Origin header (or its Referer, when there is no Origin) must name the host the request was addressed to, port included, or web.publicBaseUrl's host; Sec-Fetch-Site: cross-site and an Origin that is not a URL (null) are refused with 403. This is checked before the body is read and before anything counts as an attempt. A request with neither header (a script) passes. The sign-in page sets <meta name="referrer" content="same-origin">, because under the response header's no-referrer policy a browser sends Origin: null with the form's post.
• That same-site check has its own refusal gate, TokenSignInSite. Its log line reads "same-site check on the token sign-in form (set web.publicBaseUrl behind a proxy that rewrites Host)", it no longer reads as the OIDC flow, and its rate-limit key is separate from the OIDC flow's and from the wrong-token gate's.
• A sign-in POST that carries Sec-Fetch-Site: same-origin is accepted. This keeps sign-in working behind a proxy that rewrites Host when web.publicBaseUrl is not set. A POST without that header is judged by its Origin or Referer against the request host, the X-Forwarded-Host of a trusted proxy, or web.publicBaseUrl. The 403 text and the README name web.publicBaseUrl.
• Host names in that comparison are compared in their ASCII (punycode) form, so a Unicode name and its punycode form are the same name.
• The 4 KB sign-in body cap applies to the bytes read, so chunked bodies are held to it too. Only application/x-www-form-urlencoded is read; a multipart body is refused unread.
• ConfigurePipeline takes an optional utcNow clock for the sign-in wait, as the MCP host does. The web wait tests use a settable clock, so they cover the wait expiring without sleeping.
• With trustedProxies set, only the address of the connection itself can be loopback for allowFrom. A forwarded 127.0.0.1 or ::1 is tested against allowFrom like any other address, and a proxy on this host that forwards a client outside allowFrom still gets 403. The proxy must append the client's address to X-Forwarded-For or overwrite it, and trustedProxies must not cover clients that connect directly.
• web.network.trustedProxies lists the proxies whose forwarded scheme and address are used. The list is parsed before a certificate is loaded, so a bad entry never publishes one. A proxied HTTPS sign-in gets a Secure cookie.
• trustedProxies entries follow the same plain-text rule as allowFrom (CidrAllowList.IsPlainCidrText). An entry is an IP or a CIDR written as four decimal numbers 0-255 with no leading zero, or an IPv6 address. An entry whose written form names a different network than the parser would read (192.168.010.0/24, 10/8) is refused, and so is a bare IP written that way (010.0.0.5, 10.1). Plain entries (10.0.0.5, 10.0.1.0/24, ::1) are accepted as before. The refusal names the entry.
• Strict-Transport-Security is sent on responses to HTTPS requests only when web.network.hsts is true (it is off by default). It is kept per host name and ignores the port: once a browser has reached the dashboard over HTTPS by name, it uses HTTPS for every port of that same name for a year. The code comment and the README say so.
• A token under 16 characters, or with fewer than 6 different characters, keeps the dashboard loopback-only (still gated by that token) and logs at Critical. A blank token still stops the start.
• The --configure-network hints, --print-web-token text, darling.sample.json, the Darling README and docs/uat-onboarding.md describe the sign-in form instead of a URL token. In the Darling README, the web sign-in text is one paragraph that matches the code, the MCP certificate and allowFrom paragraphs are back in the MCP section, and the two compose paragraphs are restored.

MCP host

• A bearer token under 16 characters keeps the host loopback-only (still gated by that token). With mcp.allowWrites on, such a token leaves the write tools off.
• Wrong bearer tokens from one address wait longer each time, with the same 429 / Retry-After shape as the dashboard. A request with no bearer header, or with one that is not a bearer, does not count and does not take part in the wait.
• The check and the count are one atomic step before the token is compared, as on the dashboard: a burst of parallel requests with wrong tokens compares exactly the allowance and the rest get 429. A request that is held up only by requests from the same address that are still being checked (a client that sends several requests at once) waits for them, at most 5 seconds, instead of getting 429: parallel requests with the right token are all served, and if the ones it waited on were wrong, it is held without a comparison. During the wait no token is compared, on the loopback bind too (there every local client is one address, and every loopback address, 127.0.0.0/8 and ::1, is that one address), and a correct token is refused with the rest. A success gives back only its own attempt. The wait is kept in memory: a service restart clears it, and the 429 text and the README say so. IPv6 sources are grouped by /64 and the same total budget applies, to sources that have not presented a correct token during the last hour; a source that did is not held by the total budget, and its own wait still applies. When the total budget holds, the 429 text and the refusal line say so and say that a service restart clears it. The MCP host reads no forwarded headers, so behind a proxy every client is one address; the README says so.
• The server-tag tools (create_, update_, delete_, assign_, unassign_server_tag) are in the write-tool set, so they are served only when writes are on. Every registered tool is classified: the write-policy tests list all registered tools and fail unless each is in the write set or on an explicit read-only list, so a new tool fails until someone classifies it. Every write tool carries readOnlyHint false and destructiveHint true; every other tool carries readOnlyHint true.
• edit_server is a write tool and a server-admin tool: it is served only with mcp.allowWrites and mcp.allowServerAdmin, like add_servers and remove_server. Before it probes, it applies the add_servers screen to the definition as it would be saved, so a change that points Windows authentication at an unverified connection is refused (unless mcp.allowWindowsAuthOverUnverifiedTls is true) and nothing is probed or saved. The dashboard's edit route passes no policy.
• The start path's write and token decisions are single functions (ResolveForStart, RequireTokenWhenLoopbackOnly, ApplyTokenFloor, CreateHostBuilder) that the host and the tests both call. The log for a token under 16 characters says it is shorter than the minimum and no longer gives its length. The README gives a PowerShell command that makes a strong token with a cryptographic random generator.
• add_servers refuses Windows authentication over an unverified connection (trust server certificate, or Optional encryption) unless mcp.allowWindowsAuthOverUnverifiedTls is true. The refusal happens before the store or the server is touched.
• add_servers has an overload for callers that are not MCP clients; the dashboard's add route uses it.
• The support bundle's config shape reports the three MCP switches as mcp_allow_writes, mcp_allow_server_admin and mcp_allow_windows_auth_over_unverified_tls (booleans only), and the reflection test that classifies every config property lists them as projected.

Lite

• mcp_allow_writes setting, a write policy and the unmute_analysis_finding tool (served only when writes are on). Tool counts in the instructions, README and llms.txt are 90 (Lite) and 181 (Darling).
• Lite's MCP endpoint has no token or caller check, so the two write tools stay off by default, and the mcp_allow_writes comment in settings.sample.json and the README say that turning it on lets any program on this computer mute or unmute findings. Lite's host registration (RegisterMcpServer) is one method the tests also call.
• The Lite instructions text was shortened to stay inside its 8,000-character budget.

Compose and docs

• The compose files publish the service ports on loopback.

Already on dev

These protections already exist on dev, so dev's version is kept and nothing is carried:

• Browser response headers (a policy with frame-ancestors 'none', X-Frame-Options, nosniff, Referrer-Policy). Only HSTS is added, inside dev's middleware.
• TLS 1.2/1.3 floor and intermediate chain on both listeners (dev's shared helper).
• add_servers generic connect-failure reply, with the driver text in the service log.
• env: / file: references refused from web and MCP password fields; password taken as given.
• Plain-HTTP network listener notice (dev's log level, Warning, is kept; the compose file and darling.sample.json now say Warning instead of Critical).

Test plan

Latest run, at c3bd70ced:

• The support bundle's config shape now reports the three MCP switches. DiagnosticsBundle* classes: 215 total, 26 skipped (live), 0 failed; *Census* 500 and *Ratchet* 15, 0 failed; -trait Stage=Guard 1,517 passed. Lite: CollectionHealthRunTimeToolTests 19 passed (it fails with _scheduleManager removed from the host call), Census 67, -trait Stage=Guard 496 passed.

Earlier run, at 9b9a3c8e8:

• Web host, sign-in, sign-in hardening, host header, host binding, write policy, bearer throttle and MCP host classes: 822 passed, 0 failed.
• Every *Census* and *Ratchet* class in Darling.Tests: 515 total, 1 skipped, 0 failed. -trait Stage=Guard: 1,517 passed.
• ServerEditLiveTests on a throwaway local PostgreSQL: 11 total, 1 skipped (the one that needs a real SQL Server), 0 failed.
• Lite.Tests McpWritePolicy, Census and Ratchet classes: 79 passed. -trait Stage=Guard: 496 passed.
• The sign-in loop test (AWrongTokenThenTheRightOne_LandsWhereAFirstTrySuccessWould, 5 cases: a wrong token, a second wrong token, then the right one lands where a first-try success does) was checked against the old code. With the returnPath pass to WriteLoginPageAsync and the page-script condition removed, all 5 cases fail (the form carries an empty return); restored, all 5 pass.
• Not run here: a browser check of the sign-in form, live classes other than ServerEditLiveTests, Installer.Tests, the full suite. CI decides.

Earlier runs, before f964fa391 and 9b9a3c8e8 (counts there are older than the lines above):

• Darling.Tests and Lite.Tests build with 0 warnings, 0 errors.
• Darling targeted classes (everything touched, every *Census* and *Ratchet* class, the MCP and web host classes, compose and config pins): 2,666 run, 0 failed, 293 skipped (live classes need a database).
• Lite *Mcp*, *Census*, *Ratchet*, inventory pins: 827 run, 0 failed.
• MCP tools/list budget: measured 95,222 bytes (dev 94,429 + 793 for unmute_analysis_finding); the ceiling in McpToolsListBudgetTests is 95,222.
• Sign-in wait, bearer wait and web host classes (DarlingSignInThrottleTests, DarlingMcpBearerThrottleTests, DarlingWebSignInHardeningTests, DarlingWebHostGateLiveTests, DarlingWebAuthTests, DarlingHttpRefusalLogTests, DarlingHostBindingTests): 408 run, 0 failed (repeated 5 times, same result). They include a 500-way and a 64-way and a 16-way concurrent burst that reserve exactly the allowance, 16 concurrent right tokens that are all served (MCP on both binds, and the dashboard sign-in plain and staged), a mix of right and wrong tokens (14 right with 2 wrong are all decided as sent; 8 and 8 compare exactly 4 wrong ones), the wait on in-flight reservations (admitted when one matches, held without a comparison when they miss, bounded, cancellable), sources that spent the total budget not stopping a source with a recent success on MCP and on the dashboard (whose own wait still holds it), the expiry and bound of that memory, 127.0.0.1, 127.0.0.2, ::1 and mapped loopback sharing one source, the total-budget 429 text, a staged burst of 40 held open and released together, the origin check, the chunked and multipart body cases, a forwarded loopback address, the /64 grouping, the total budget, and pins for the compose level and the HSTS wording.
• Tool classification, write-switch behavior (writes off: create_mute_rule and edit_server on / and /core, Lite's mute and unmute, are refused as unknown tools; a weak or missing token leaves writes off), edit_server screen and token-floor decision tests: Darling write-policy, edit-core, bearer-throttle and host-binding classes plus Census, Ratchet, tool-list budget and Stage=Guard classes: 3,316 run, 0 failed, 329 skipped (live). Lite Mcp write-policy, Census, Ratchet, budget classes: 86 run, 0 failed; Stage=Guard 496 run, 0 failed.
• After the sign-in wait change: *Census* and *Ratchet* 515 run, 0 failed, 1 skipped; the MCP and web host classes, tool-list budget and instructions-budget classes together with those and the throttle classes: 1,127 run, 0 failed, 1 skipped; -trait Stage=Guard 1,517 run, 0 failed.
• Sign-in origin check, IDN host names, HSTS setting, token floor: web host, sign-in, bearer-wait, host-binding, write-policy and host classes 414 run (the one failure was a test that expected HSTS by default, now set to opt in), then DarlingWebSignInHardeningTests 59 run, 0 failed. ServerEditLiveTests 11 run, 0 failed, 1 skipped on a throwaway local PostgreSQL; the new tool-path test fails when policy: policy is dropped at either call and passes when restored. Edit-core, route, tools, Census, Ratchet, Stage=Guard filters together: 418 run, 0 failed.

Changelog

SECTION: Changed
ENTRY:

• A token in the web dashboard's address no longer signs anyone in; the dashboard and MCP host take the access token only from the sign-in form or bearer header, with a minimum length and a wait after failures ([MCP and web host defaults: form sign-in, token length floor, sign-in wait, write-tool token gate #5702]) - the dashboard takes its token only from the sign-in form's POST to /auth/login. A token in the address (?token=) is never compared: a bookmark or link that carries one opens the sign-in form, and the service log says why. A script posts the token to /auth/login and then presents the session cookie it gets back. A wrong token keeps the page you were opening, so a right token on the next try lands there. A token under 16 characters, or with fewer than 6 different characters, keeps either host loopback-only. Repeated wrong tokens from one address wait longer (429 with Retry-After). trustedProxies supports HTTPS behind a proxy and accepts only plain IP or CIDR text, and web.network.hsts turns HSTS on. The server-tag tools are write tools. add_servers and edit_server ask for mcp.allowWindowsAuthOverUnverifiedTls before accepting Windows authentication over an unverified connection, and edit_server is a server-admin write tool that applies the same check. Lite gains mcp_allow_writes and unmute_analysis_finding.
  REF:
  [MCP and web host defaults: form sign-in, token length floor, sign-in wait, write-tool token gate #5702]: MCP and web host defaults: form sign-in, token length floor, sign-in wait, write-tool token gate #5702

[claude]

One mistyped token locks the operator into a sign-in loop.

This arm re-renders the login page as the response to POST /auth/login, so the browser's address bar is now /auth/login. The page's own script fills the hidden field from the current location:

document.getElementById('return').value = location.pathname + location.search + location.hash;   // :2413

so the retry posts return=/auth/login. The retry with the correct token then runs line 2135, SanitizeRedirectPath("/auth/login") → /auth/login, and 302s there. /auth/login is now an auth-flow path in every mode (IsAuthFlowPath, :1528) and DecideWebRequest returns HandleAuthFlow before it looks at the session cookie (:1517-1523), so the GET lands back in HandleTokenSignInAsync, which on a non-POST writes the login form again (:2073) without ever checking for the cookie it just minted.

Failure scenario, start to finish:

1. Open http://host:5153/alerts, no cookie → login page at /alerts, return=/alerts.
2. Mistype the token → POST /auth/login → 200 login page, URL is now /auth/login.
3. Type the token correctly → 302 Location: /auth/login with Set-Cookie.
4. GET /auth/login → login form again. Repeat 3–4 forever.

The operator holds a valid session cookie the whole time and has no way to see it; the only escape is hand-editing the URL. A mistyped paste is exactly the case the new throttle is built around, so this is the common path, not a corner. No test covers it — every PostSignIn helper either omits return (→ /) or passes an explicit path (DarlingWebHostGateLiveTests.cs:231).

Two independent fixes, either of which closes it: have the GET leg of HandleTokenSignInAsync 302 to / (or to return) when TryValidateSessionCookie already passes, and/or have SanitizeRedirectPath's sign-in caller map an auth-flow path to / so a success never lands back on the form.

[claude]

trustedProxies parses with bare IPNetwork.TryParse, so inet_aton spellings silently widen the trust list.

CidrAllowList exists because this repo already learned that IPNetwork.TryParse alone is not a CIDR reader — it follows inet_aton. The repo's own assertions prove it (Darling.Tests/DarlingCidrAllowListTests.cs:140-154):

Assert.Equal("192.168.8.0/24", IPNetwork.Parse("192.168.010.0/24").ToString()); // leading zero = octal
Assert.Equal("0.0.0.0/8",      IPNetwork.Parse("10/8").ToString());             // short form zero-pads
Assert.Equal("10.0.0.0/8",     IPNetwork.Parse("0x0A.0.0.0/8").ToString());     // 0x = hex

CidrAllowList.TryParse guards against exactly those with IsPlainCidrText ("so the range written is the range enforced"), and refuses an IPv4-mapped entry. TryParseTrustedProxies calls neither, so:

• "trustedProxies": ["192.168.010.0/24"] is accepted and honors forwarded headers from 192.168.8.0/24 — a block the operator never wrote, while the block they meant is not trusted.
• "trustedProxies": ["10/8"] is accepted as 0.0.0.0/8.

This list is strictly more dangerous than allowFrom: a source inside it can set X-Forwarded-For to any value, which this PR then feeds to IsInAllowFrom (DarlingWebHostService.cs:1555) and to DarlingSignInThrottle.SourceKey, i.e. it bypasses allowFrom and resets the sign-in wait per spoofed address; X-Forwarded-Proto from it also decides the cookie's Secure flag and HSTS. It fails open in the direction that matters, and the Critical line an operator would need never fires because the entry parses.

TryParseTrustedProxies_RefusesAnEntryItCannotRead only covers proxy.corp.local; nothing pins the spellings above. Routing each entry through CidrAllowList.IsPlainCidrText (and the same check on the bare-IP fallback at :310) would reuse the rule the rest of the host already enforces.

[claude]

This refusal is logged under the OIDC gate, on a dashboard that may have no OIDC at all.

DarlingRefusalGate.SignIn is the OIDC flow's gate — its doc comment says so, and DarlingHttpRefusalLog.Describe renders it verbatim as "OIDC sign-in flow (web.network.oidc)" (DarlingHttpRefusalLog.cs:36-43, 275). The refusal here is the shared-token form post's same-site check, which exists in every mode. An operator whose darling.json has no oidc block reads a Warning naming a gate they have not configured, on a class whose whole job is "which gate refused it, named the way an operator would have to name it to fix it".

It also shares the rate limiter's fold key. The budget is per (gate, source), so a scanner's cross-site posts and a genuine OIDC callback failure from the same address collapse into one bucket and one can hide the other — the precise reason ReadOnlySeat was given its own gate rather than reusing Token (DarlingHttpRefusalLog.cs:30-34).

A TokenSignIn member (or reusing Token, which is already what the other three refusals in this method use) would keep the label true and the fold key separate.

[claude]

Non-blocking, but worth matching the MCP side: the web sign-in throttle reads DateTime.UtcNow directly, while DarlingMcpHostService.ConfigurePipeline took a Func<DateTime>? utcNow seam for exactly this ("A test passes a settable one so the wait can be crossed without sleeping"). DarlingWebHostService.ConfigurePipeline did not, so every web wait test is pinned to real elapsed time.

BaseDelay is 2 s, and RepeatedWrongTokens_FromOneSource_AreHeldWithRetryAfter_… / ARightToken_GivesBackOnlyItsOwnAttempt_… assert that two sequential PostSignIn calls land inside that window. In-process through TestServer that is normally microseconds, but a loaded runner that stalls either await past 2 s flips the expected 429 to a 302/200 — the #3676 shape in .github/REVIEW-TRAPS.md ("a test that reads the clock twice measures the CI scheduler"). Threading the same utcNow parameter through ConfigurePipeline → HandleAuthFlowAsync → HandleTokenSignInAsync would remove the dependency and also let a test cross the wait and assert it expires, which nothing does today.

[claude]

Verdict: CHANGES REQUESTED

Claims

• VERIFIED — A token is read only from POST /auth/login; ?token= is never compared — DarlingWebHostService.cs:1261 passes hasValidToken: false; tokenInUrl (:1256) feeds only refusals.Report.
• VERIFIED — 3 free failures, 2 s doubling to 5 min, reserved-then-compared in one atomic step after the body is read — DarlingSignInThrottle.DelayAfter / BeginAttempt (single lock), HandleTokenSignInAsync:2103-2110; ManyConcurrentWrongTokens_…(staged) pins FreeFailures + 1 compared out of 40.
• VERIFIED — A success returns only its own reservation; IPv6 by /64; 60/hour across all sources — Accepted, SourceKey, GlobalMaxAttempts; ARightToken_GivesBackOnlyItsOwnAttempt_….
• VERIFIED — Same-site check precedes the body read and the count, and the login page sets <meta name='referrer' content='same-origin'> — :2078 before :2089/:2108; meta at :2369.
• VERIFIED — 4 KB cap on bytes read, url-encoded only, multipart unread — ReadLoginFormAsync / IsUrlEncodedForm; ABodyOverTheCap_…, AMultipartBody_IsNotReadAtAll.
• VERIFIED — Only the connection's own address can be loopback, and trustedProxies parses before the certificate loads — DarlingConnectionAddressStamp + IsLoopbackConnection (:1573); parse at :634, DarlingListenerTls.Resolve at :664.
• VERIFIED — A sub-16-character token keeps either host loopback-only while still gating it, and leaves MCP writes off — ApplyTokenFloor at :517 with requireTokenWhenLoopbackOnly recomputed at :858, DarlingMcpHostService.cs:441, DarlingMcpWritePolicy.ResolveForStart.
• VERIFIED — Server-tag tools are writes; edit_server is a server-admin write applying the add_servers screen before any probe; add_servers screens before the store opens — DarlingMcpWritePolicy.WriteTools/ServerAdminTools, Edit.cs:347-356 (inside NeedsProbe, which connectionChanged sets on any auth/encrypt/trust change, :940-1009), DarlingMcpServerAdminTools.cs:225.
• VERIFIED — Lite gains mcp_allow_writes, a write policy and unmute_analysis_finding; the count is 90 — 90 McpServerTool(Name attributes under Lite/Mcp/; McpSettings.cs:142-150 kind-checks the key; FindingStore.UnmuteStoryAsync is the one AcquireWriteLock site and FindingStoreTests pins it there with its reason.
• VERIFIED — Compose publishes both ports on the docker host's loopback and narrows allowFrom off 0.0.0.0/0 — compose/docker-compose.yml:162-163, compose/darling.sample.json.
• UNVERIFIED — Every test-plan count (2,666 / 1,517 / 827 / 496 / 376 / 3,316 / 86 run, 0 failed) and "0 warnings, 0 errors" — no Windows/.NET 10 build or xUnit runner on this tree.
• UNVERIFIED — "measured 95,222 bytes (dev 94,429 + 793)" — that is McpToolsListBudgetTests.Measure() output; the constant and the pinned per-tool bytes agree with each other, which is not the measurement.
• UNVERIFIED — "under the response header's no-referrer policy a browser sends Origin: null", and that the <meta name=referrer> overrides the header — browser behavior; TheLoginPage_KeepsTheRealOriginOnItsOwnPost can only assert markup. If the header wins in a shipping browser, IsSameSiteSignIn 403s every browser sign-in.
• UNVERIFIED — HSTS "uses HTTPS for every port of that same name for a year" — correct per RFC 6797 on paper, but a live browser behavior; the test pins the header value and the wording only.
• REFUTED — "the sign-in leg 302s back to the page the form was covering", as Darling/README.md and docs/uat-onboarding.md now promise operators — true only on a first-try success. After a wrong token the form is re-served as the response to POST /auth/login (:2129), the page's own script (:2413) then fills return with /auth/login, and the correct retry 302s there — a path IsAuthFlowPath (:1528) routes to HandleTokenSignInAsync ahead of the cookie check (:1517-1523), so the form renders again and the operator never lands. Inline comment on :2129.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingWebHostService.cs:2129 — re-rendering the form at /auth/login is what makes return self-referential and loops every later correct sign-in — UNPINNED (every PostSignIn helper omits return or passes an explicit path, DarlingWebHostGateLiveTests.cs:231)
2. Darling/PerformanceMonitor.Darling.Service/Hosting/DarlingHostBinding.cs:306 — the trusted-proxy list is read with bare IPNetwork.TryParse, skipping the IsPlainCidrText rule the rest of the host enforces; a trusted source's X-Forwarded-For is what allowFrom and the throttle then judge — UNPINNED (TryParseTrustedProxies_RefusesAnEntryItCannotRead covers only a hostname)
3. Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingWebHostService.cs:1261 — hasValidToken: false ends the URL-token exchange for every bookmark, script and sign-in link in the field, and leaves SetCookieAndRedirect dead — pinned by GetOnTheSignInPath_RendersTheForm_AndAUrlTokenThereIsNotExchanged and DarlingWebAuthTests

Traps near this diff

• "Lite and Darling ship in parity (CONTRIBUTING's two-store rule): a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit" — does not apply — unmute_analysis_finding is Lite-only on purpose (Darling's MCP role holds INSERT-only on that table: DarlingManagedRoles.cs:1266, provision-roles.sql:709), the reason is inline in KnownDarlingMissingMcpTools, and the new LiteOnlyMcpTools_MatchTheAllowList holds the carve-out to an exact set. The write switch itself landed on both SKUs.
• "A test that reads the clock twice measures the CI scheduler … One anchor owns both ends of an interval" (#3676) — applies — the MCP host took a Func<DateTime>? utcNow seam, DarlingWebHostService.ConfigurePipeline did not, so :2061/:2107 read the real clock and the web wait pins need two sequential TestServer calls inside a real 2-second window. Non-blocking; inline comment on :2107.
• "A pin that encodes the lie … Ask what a pin would fail ON" (#3699) — does not apply — DarlingWebSignInHardeningTests.cs:426 fails if BeginAttempt stops being atomic and :461-464 fails if Accepted clears more than one reservation; neither asserts its own premise.
• "A roster-based census does not see the file it does not roster … An anti-pattern census sweeps the class" (#3699) — does not apply — McpWritePolicyTests / DarlingMcpWritePolicyTests enumerate every registered tool from the host's own registration and fail on any name in neither WriteTools nor McpToolClassification.ReadOnlyTools.
• ".gitattributes says text=auto eol=crlf … A diff that touches every line of a file is a line-ending conversion until proven otherwise" — does not apply — the one whole-block rewrite, Lite/Mcp/McpHostService.cs, is a real extraction: the deleted lines reappear verbatim in RegisterMcpServer (Lite/Mcp/McpHostService.cs:154-233).

Divergence (blind read vs PR body)

Three places where the accounts part.

1. Where a success lands. The body details the wait, the counting and the 429 but never says where a success goes; the README and docs/uat-onboarding.md this PR rewrites both promise the page you were opening. The diff delivers that only on a first-try success. After any miss the operator is looped on /auth/login — the one path that renders the login form regardless of a valid session cookie — and a mistyped paste is the exact case the throttle exists for. Inline on :2129.

2. What "an entry that does not parse" covers for trustedProxies. True for entries IPNetwork.TryParse rejects. Unsaid: this parser accepts spellings resolving to a different network than the one written — the repo's own DarlingCidrAllowListTests.cs:140-154 asserts 192.168.010.0/24 → 192.168.8.0/24 and 10/8 → 0.0.0.0/8, which is why CidrAllowList has IsPlainCidrText and allowFrom refuses them. Inline on DarlingHostBinding.cs:306.

3. The gate the cross-site refusal logs under. Unmentioned in the body; the diff reuses DarlingRefusalGate.SignIn, which Describe renders as "OIDC sign-in flow (web.network.oidc)" and which shares the limiter's (gate, source) fold key with real OIDC refusals. Inline on :2083.

Everything else matched what I found before reading the body, including the parts I expected to be overstated: the reserve-before-compare really is one lock, the add_servers screen really runs before the store opens, and the write switch really landed on both SKUs.

Reviewer checklist

• In a real browser on an exposed dashboard: open a deep link, mistype the token once, then enter it correctly, and confirm you reach the page rather than the form again. Repeat with OIDC on.
• In Chrome and Firefox, confirm the form's POST carries a real Origin (not null) — i.e. the meta tag beats the Referrer-Policy: no-referrer header. If it does not, every browser sign-in 403s.
• Behind an actual TLS-terminating proxy in trustedProxies: Secure cookie, https:// OIDC redirect_uri, HSTS present, allowFrom judging the forwarded client, and a direct connection's spoofed X-Forwarded-For ignored.
• Run the suite on Windows/.NET 10 for the test-plan counts, the 95,222-byte tools/list measurement, and the live classes skipped here.
• Upgrade a Darling install that used the MCP write tools: the Information line at start, the tools returning once mcp.allowWrites (+ mcp.allowServerAdmin) and a 16+ character token are set, and a pre-existing short token degrading the listener rather than stopping the service.

[claude]

Non-blocking observation: on the InFlightWaitLimit timeout path, the attempt is always held for a flat BaseDelay (2s) with SignInHold.Source, regardless of how large entry.Misses + entry.InFlight has actually grown (the value Decide used just above to compute until, at line 189-190). If several in-flight reservations from the same source stack up past FreeFailures + 1 and then genuinely stall (thread-pool starvation, a hung comparison), every attempt that times out here reports a 2-second Retry-After while the real per-source backoff for that failure count (DelayAfter(entry.Misses + entry.InFlight)) would be larger. The client is told to retry sooner than the state actually allows, so it just loops back into another 5-second Pending wait. No reservation or comparison is affected (state isn't touched here), so this is a cosmetic Retry-After inaccuracy in a rare stall scenario, not a security or correctness issue — flagging since none of the new tests exercise entry.Misses + entry.InFlight above FreeFailures + 1 at the point InFlightWaitLimit expires.

[claude]

Verdict: LGTM

Claims

• VERIFIED — "parallel requests from one client wait for each other" instead of 429ing: in-flight reservations gate a new attempt into SignInAttempt.Pending() (DarlingSignInThrottle.cs Decide, the entry.InFlight > 0 branch), and BeginAttemptAsync loops on it, bounded by InFlightWaitLimit — pinned by SixteenConcurrentRightTokens_AreAllServed and SixteenConcurrentRightTokens_FromOneSource_AllSignIn (staged and unstaged).
• VERIFIED — "a source that presented the right token is not held by the total budget" — Accepted calls RememberSuccess, and Decide's total-budget check is skipped when _recentSuccess.ContainsKey(source) — pinned by ASourceWithARecentSuccess_IsNotHeldByTheTotalBudget_ButStillByItsOwnWait and the MCP/web OtherSourcesSpendingTheTotalBudget_DoNotStop… integration tests, which also confirm the source's own wait still applies afterward.
• VERIFIED — "every loopback address is one source" — SourceKey now returns LoopbackKey for IPAddress.IsLoopback(ip) before the /64 branch — pinned by EveryLoopbackAddress_IsOneSource (unit) and EveryLoopbackAddress_ShareOneWait_OnTheLoopbackBind (MCP integration, four spellings including an IPv4-mapped IPv6 one).
• VERIFIED — "the total-budget 429 says so" — SignInHold.Total vs .Source selects distinct message text in both DarlingMcpHostService.cs and DarlingWebHostService.cs ("not necessarily this one" / "No token is compared from an address…" vs "from this address…") — pinned by the body-substring assertions in both new OtherSourcesSpendingTheTotalBudget_… tests.
• VERIFIED — README wording was updated to match (in-flight wait, four-compared-per-address ceiling, loopback folding, the recent-success carve-out, the total-hold explanation) — Darling/README.md diff reads consistently with the code above.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Service/Hosting/DarlingSignInThrottle.cs:202 — BeginAttemptAsync's InFlightWaitLimit timeout path returns a flat Held(BaseDelay, Source) rather than DelayAfter(entry.Misses + entry.InFlight) — UNPINNED for the case where that sum has grown past FreeFailures + 1 by the time the wait times out (cosmetic Retry-After understatement only; inline comment posted).
2. Darling/PerformanceMonitor.Darling.Service/Hosting/DarlingSignInThrottle.cs:173-213 (Decide) — the single lock-guarded state machine now carries three outcomes (held/pending/reserved) plus in-flight bookkeeping shared by Accepted/Missed/Signal — pinned thoroughly by DarlingSignInThrottleTests' new concurrent-async tests (ManyConcurrentAsyncAttempts_AllMissing_…, …AllMatching_…, AMixWithinTheAllowance_…, AnAttemptOnlyHeldUpByReservationsInFlight_…, AnAttemptWaitingOnReservationsInFlight_…, TheWaitOnReservationsInFlight_IsBounded_AndCancellable).
3. Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs:1787 / DarlingWebHostService.cs:1335 — switching the gate to await BeginAttemptAsync(...) means a client's RequestAborted can now surface as OperationCanceledException from inside what used to be a synchronous, non-awaiting check — pinned by TheWaitOnReservationsInFlight_IsBounded_AndCancellable at the throttle level; no new MCP/web integration test cancels mid-wait, but Kestrel's standard handling of request-aborted cancellation elsewhere in this codebase makes this low risk.

Traps near this diff

• "Lite and Darling ship in parity (CONTRIBUTING's two-store rule): a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding" — does not apply — DarlingSignInThrottle and the bearer/sign-in gates it backs are Darling-only surfaces (Lite is the WPF desktop app with no networked MCP/web sign-in; grep for SignInThrottle/BeginAttempt under Lite/ finds nothing), so there is no Lite counterpart for this change to diverge from.
• "A test that reads the clock twice measures the CI scheduler … One anchor owns both ends of an interval" (#3676) — does not apply to the new tests — the new throttle-level tests drive time through the injected Clock/fixed T0, and the new async in-flight tests (Task.Delay(50), WaitAsync(TimeSpan.FromSeconds(10))) assert only "not yet completed" / "eventually completes", not a value computed from two separate real-clock reads.

Divergence (blind read vs PR body)

None — the commit messages ("parallel requests from one client wait for each other", "a source that presented the right token is not held by the total budget", "every loopback address is one source", "the total-budget 429 says so") match the diff and the test coverage on every point graded above. This follow-up does not touch the three issues raised in the prior full review (the /auth/login redirect loop, the trusted-proxy CIDR parsing, and the URL-token exchange) — none of the lines involved in those findings are part of this diff.

Reviewer checklist

• Run Darling.Tests (the three throttle/bearer/sign-in classes touched here) on the real CLR: the new concurrent-async tests use real thread-pool scheduling and Task.Delay/WaitAsync timing that a read-only tree cannot execute.
• Under real load, confirm the InFlightWaitLimit (5s) default doesn't hold HTTP connections open long enough to matter for the web dashboard's or MCP client's own request timeout, now that a burst of same-source requests can block inside the gate instead of returning immediately.

Claude posting for Erik Darling. Answered in f076432; the bot's LGTM at f076432 followed it.