Fix a round of correctness/security issues across the enterprise features

Uncommitted work from a parallel session, reviewed and verified here
(full build, 144 tests, fmt, clippy all clean) before committing:

- aria-describedby/aria-errormessage hold a space-separated *list* of
  ids; a single getElementById on the whole value returned null for a
  multi-id list, wrongly failing a correctly-described field.
- Baseline::from_runs kept the first status seen for a repeated URL
  instead of the newest, so a baseline built from unsorted/multi-run
  input could record a stale severity.
- doctor's audit-log probe checked the log's parent directory even
  when the log file itself already existed and was read-only, so it
  could report writable while every real append failed.
- limiter::host_of split IPv6 literals on ':', collapsing every one
  onto a single pacing bucket ("[" for all of them).
- notify::is_slack_webhook matched the URL by substring, so
  "https://example.com/?next=hooks.slack.com" was misclassified as a
  Slack webhook; now matches the host exactly via limiter::host_of.
- Chrome-open backoff and the wizard/persistence timeout multiplier
  could overflow and panic on a large --check-timeout-secs; both
  saturate now.
- export's XML escaper dropped control characters but not the other
  two characters XML 1.0 forbids (U+FFFE/U+FFFF).
- serve's /readyz reported ready via create_dir_all alone, which
  returns Ok for an existing-but-read-only directory; now probes with
  an actual write. The server also bounds concurrent connections (a
  semaphore) and caps header-read time, so a slow/malicious client
  can't exhaust it with unbounded half-open connections.
- LLM hardening: retries now apply to 429/5xx whose body isn't JSON
  (read as text before parsing, so a non-JSON error body no longer
  turns a retryable status into a permanent failure); Retry-After
  honors its full documented 60s cap instead of being re-capped to 8s;
  the verdict cache key includes the resolved base URL so switching
  --llm-base-url can't serve a stale backend's verdict; the Anthropic
  parser finds the first *text* block instead of assuming index 0 (a
  thinking/tool-use block can precede it); verdict parsing tries every
  balanced {...} region instead of first-brace-to-last-brace, so prose
  containing its own braces no longer discards a valid verdict
  further along; and LlmOptions gets a hand-written Debug that
  redacts the API key instead of the derive printing it verbatim.
- history::load_runs now breaks same-second-timestamp ties by
  filename, so "the previous run" (and the diff/streak/flakiness built
  from it) can't flip between two calls depending on read_dir's
  unspecified order.
- report --prometheus's formwatch_forms_total carried the reserved
  counter _total suffix on what's actually a gauge, which promtool
  rejects; renamed to formwatch_forms.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: voidstackloop

CHANGELOG.md

@@ -64,7 +64,7 @@ project doesn't have a release yet, so everything below is grouped under
 - `test` and `monitor` accept `--junit` / `--sarif` (with `--out`), so a
   single CI job can run the checks and emit results in one step.
 - `report --prometheus` renders Prometheus text-exposition metrics
-  (`formwatch_forms_total`, `formwatch_checks_failing`,
+  (`formwatch_forms`, `formwatch_checks_failing`,
   `formwatch_checks_warning`, `formwatch_check_last_run_timestamp_seconds`,
   and per-check `formwatch_check_status`) for a scrape endpoint or a
   node_exporter textfile-collector directory.
@@ -188,6 +188,49 @@ project doesn't have a release yet, so everything below is grouped under
 
 ### Fixed
 
+- Validation reported a false `FAIL` for a correctly-described field whose
+  `aria-describedby` (or `aria-errormessage`) listed more than one id: the
+  check resolved the whole space-separated list with a single
+  `getElementById`, which returns null for a list. Each id is now
+  resolved, and either attribute counts as a described field.
+- LLM verdict parsing clamped an out-of-range score up to a passing 5,
+  contradicting the module's own contract that an out-of-range score is a
+  parse failure and letting a model-injected `{"score": 99}` force a
+  `Pass`. It is now rejected (the check degrades to `Warn`), and verdict
+  extraction is brace-balanced, so prose containing `{}` no longer
+  discards a valid verdict.
+- LLM retries now apply to 429/5xx responses whose body isn't JSON
+  (routine for gateway/proxy errors): the status is checked before the
+  body is parsed, instead of the failed parse turning a retryable status
+  into a permanent failure.
+- LLM `Retry-After` is honored up to its documented 60s cap instead of
+  being silently re-capped to 8s. `LlmOptions`' hand-written `Debug`
+  redacts the API key; the verdict cache key includes the resolved base
+  URL (so switching `--llm-base-url` no longer serves the previous
+  backend's verdicts); and the Anthropic response parser finds the first
+  `text` content block rather than assuming the first block is text.
+- `serve`'s `/readyz` reported ready for an existing but read-only history
+  directory — `create_dir_all` returns `Ok` for any existing directory —
+  so it now probes writability, and the server bounds concurrent
+  connections and caps header-read time.
+- `formwatch baseline --write` (`Baseline::from_runs`) recorded the
+  *first* status seen for a repeated URL instead of the newest.
+- `formwatch doctor` probed the audit log's parent directory rather than
+  an existing (possibly read-only) log file, so it could report writable
+  while every append failed.
+- `--per-host-delay-ms` collapsed every IPv6 literal onto one pacing
+  bucket: `host_of` split `[::1]:8080` on `:` and returned `"["`.
+- `--check-timeout-secs` (via the wizard timeout) and the Chrome-open
+  retry backoff could panic on overflow; both saturate now.
+- Webhook auto-detection treated any URL *containing* `hooks.slack.com`
+  (even in a query string or a look-alike host) as a Slack webhook; it
+  now matches the host exactly.
+- `report --prometheus`'s `formwatch_forms_total` was a gauge carrying the
+  reserved counter `_total` suffix, which `promtool` rejects; renamed to
+  `formwatch_forms`.
+- JUnit XML output also drops the XML-1.0-forbidden U+FFFE/U+FFFF
+  characters, and history loads same-second runs in a stable filename
+  order so "the previous run" can't vary between invocations.
 - The declared MSRV was wrong: the code (edition-2024 let-chains) and
   several transitive dependencies (`home`, `icu_*` need 1.88;
   `idna_adapter` needs 1.86) require Rust **1.88**, not 1.85, so the MSRV

src/baseline.rs

@@ -72,26 +72,36 @@ impl Baseline {
     }
 
     /// Builds a baseline from the latest run of every known form, keeping
-    /// every non-Pass check.
+    /// every non-Pass check. When `runs` happens to hold more than one run
+    /// for a URL, the newest one wins for each (url, check) — keeping the
+    /// older would record a stale severity that could then absorb (or, if
+    /// more severe, wrongly uphold) a later finding.
     pub fn from_runs(runs: &[RunResult]) -> Self {
-        let mut entries: Vec<Entry> = runs
-            .iter()
-            .flat_map(|run| {
-                run.checks
-                    .iter()
-                    .filter(|c| c.status != Status::Pass)
-                    .map(|c| Entry {
-                        url: run.url.clone(),
-                        name: run.name.clone(),
-                        check: c.name.clone(),
-                        status: c.status,
-                    })
-            })
-            .collect();
+        let mut latest: std::collections::HashMap<(&str, &str), (i64, Entry)> =
+            std::collections::HashMap::new();
+        for run in runs {
+            for c in run.checks.iter().filter(|c| c.status != Status::Pass) {
+                let key = (run.url.as_str(), c.name.as_str());
+                if latest.get(&key).is_none_or(|(ts, _)| run.timestamp >= *ts) {
+                    latest.insert(
+                        key,
+                        (
+                            run.timestamp,
+                            Entry {
+                                url: run.url.clone(),
+                                name: run.name.clone(),
+                                check: c.name.clone(),
+                                status: c.status,
+                            },
+                        ),
+                    );
+                }
+            }
+        }
+        let mut entries: Vec<Entry> = latest.into_values().map(|(_, entry)| entry).collect();
         entries.sort_by(|a, b| {
             (a.url.as_str(), a.check.as_str()).cmp(&(b.url.as_str(), b.check.as_str()))
         });
-        entries.dedup_by(|a, b| a.url == b.url && a.check == b.check);
         Self {
             schema_version: BASELINE_SCHEMA_VERSION,
             generated_at: chrono::Utc::now().timestamp(),
@@ -259,6 +269,31 @@ mod tests {
         );
     }
 
+    #[test]
+    fn from_runs_keeps_the_newest_status_for_a_repeated_url() {
+        let older = RunResult {
+            schema_version: SCHEMA_VERSION,
+            name: "x".into(),
+            url: "https://a.gov".into(),
+            timestamp: 1,
+            checks: vec![check("Accessibility", Status::Fail)],
+        };
+        let newer = RunResult {
+            schema_version: SCHEMA_VERSION,
+            name: "x".into(),
+            url: "https://a.gov".into(),
+            timestamp: 2,
+            checks: vec![check("Accessibility", Status::Warn)],
+        };
+        let baseline = Baseline::from_runs(&[older, newer]);
+        assert_eq!(baseline.len(), 1);
+        assert_eq!(
+            baseline.entries[0].status,
+            Status::Warn,
+            "the newest run's status must win, not the first one seen"
+        );
+    }
+
     #[test]
     fn accepts_equal_or_better_but_not_worse() {
         let baseline = Baseline {

src/browser.rs

@@ -259,7 +259,10 @@ pub async fn open_with(browser: &Browser, url: &str, opts: &OpenOptions) -> Resu
             }
         }
         if attempt + 1 < attempts {
-            tokio::time::sleep(opts.backoff * 2u32.pow(attempt)).await;
+            // Saturating: `attempts` is a public field, and a large value
+            // would otherwise overflow `backoff * 2^n` and panic.
+            let backoff = opts.backoff.saturating_mul(2u32.saturating_pow(attempt));
+            tokio::time::sleep(backoff).await;
         }
     }
     Err(last_err.expect("loop runs at least once, always setting last_err on failure"))

src/checks.rs

@@ -729,11 +729,18 @@ pub async fn check_validation_errors(page: &Page) -> Result<CheckResult> {
                 const f = {TARGET_FORM_JS};
                 if (!f) return 0;
                 const invalid = ({DEEP_QUERY_JS})(f, 'input:invalid, select:invalid, textarea:invalid, [aria-invalid=true]');
+                const idsOf = (el, attr) => (el.getAttribute(attr) || '').split(/\\s+/).filter(Boolean);
+                const hasText = (id) => (document.getElementById(id)?.textContent || '').trim().length > 0;
                 let unlabeled = 0;
                 for (const el of invalid) {{
-                    const describedBy = el.getAttribute('aria-describedby');
-                    const hasDescribedText = describedBy && document.getElementById(describedBy)?.textContent.trim();
-                    if (!hasDescribedText) unlabeled += 1;
+                    // aria-describedby (and aria-errormessage) hold a
+                    // *space-separated list* of ids; getElementById on the
+                    // whole attribute value returns null for a multi-id
+                    // list, which used to count a correctly-described field
+                    // as failed.
+                    const described = idsOf(el, 'aria-describedby').some(hasText)
+                        || idsOf(el, 'aria-errormessage').some(hasText);
+                    if (!described) unlabeled += 1;
                 }}
                 return unlabeled;
             }})()"#

src/doctor.rs

@@ -94,12 +94,23 @@ pub fn run(ctx: &Context) -> Vec<Check> {
 
     match ctx.audit_log {
         Some(path) => {
-            let parent: PathBuf = path
-                .parent()
-                .filter(|p| !p.as_os_str().is_empty())
-                .map(Path::to_path_buf)
-                .unwrap_or_else(|| PathBuf::from("."));
-            match write_probe(&parent) {
+            // If the log file already exists, probe *it*: its parent can
+            // be writable while the file itself is read-only, in which case
+            // every later audit::append would fail despite a green doctor.
+            let probe = if path.exists() {
+                std::fs::OpenOptions::new()
+                    .append(true)
+                    .open(path)
+                    .map(|_| ())
+            } else {
+                let parent: PathBuf = path
+                    .parent()
+                    .filter(|p| !p.as_os_str().is_empty())
+                    .map(Path::to_path_buf)
+                    .unwrap_or_else(|| PathBuf::from("."));
+                write_probe(&parent)
+            };
+            match probe {
                 Ok(()) => checks.push(check(
                     "Audit log",
                     true,

src/export.rs

@@ -23,6 +23,8 @@ fn xml_escape(s: &str) -> String {
             // XML 1.0 forbids most control characters outright; drop them
             // rather than emit a document no parser will accept.
             c if (c as u32) < 0x20 && c != '\t' && c != '\n' && c != '\r' => {}
+            // U+FFFE/U+FFFF are also forbidden in XML 1.0.
+            '\u{FFFE}' | '\u{FFFF}' => {}
             other => out.push(other),
         }
     }

src/history.rs

@@ -94,22 +94,32 @@ pub fn save_run(base: &Path, run: &RunResult) -> Result<PathBuf> {
 }
 
 /// All runs for this URL, oldest first.
+///
+/// Within a single timestamp, ordering is by filename — `save_run` writes
+/// same-second runs as `<ts>-N.json`, and without that tiebreak the
+/// `read_dir` order is arbitrary, so "the previous run" (and therefore the
+/// diff, streaks, and flakiness) could flip from run to run.
 pub fn load_runs(base: &Path, url: &str) -> Result<Vec<RunResult>> {
     let dir = dir_for(base, url);
     if !dir.exists() {
         return Ok(vec![]);
     }
-    let mut runs = vec![];
+    let mut runs: Vec<(String, RunResult)> = vec![];
     for entry in fs::read_dir(&dir)? {
         let path = entry?.path();
         if path.extension().and_then(|e| e.to_str()) == Some("json") {
             let run: RunResult = serde_json::from_str(&fs::read_to_string(&path)?)
                 .with_context(|| format!("parsing {}", path.display()))?;
-            runs.push(run);
+            let file = path
+                .file_name()
+                .and_then(|n| n.to_str())
+                .unwrap_or("")
+                .to_string();
+            runs.push((file, run));
         }
     }
-    runs.sort_by_key(|r| r.timestamp);
-    Ok(runs)
+    runs.sort_by(|a, b| (a.1.timestamp, &a.0).cmp(&(b.1.timestamp, &b.0)));
+    Ok(runs.into_iter().map(|(_, run)| run).collect())
 }
 
 /// Every URL formwatch has ever recorded a run for, newest run first.

src/limiter.rs

@@ -71,8 +71,15 @@ pub fn host_of(url: &str) -> Option<String> {
     };
     let authority = rest.split(['/', '?', '#']).next().unwrap_or("");
     let authority = authority.rsplit('@').next().unwrap_or(authority);
-    // Strip userinfo already handled; strip a trailing `:port`.
-    let host = authority.split(':').next().unwrap_or("").trim();
+    // Strip a trailing `:port`. An IPv6 literal is bracketed
+    // (`[::1]:8080`), so splitting on ':' would yield "[" for every one
+    // of them and collapse all IPv6 hosts onto a single pacing bucket.
+    let host = if let Some(rest) = authority.strip_prefix('[') {
+        rest.split(']').next().unwrap_or("")
+    } else {
+        authority.split(':').next().unwrap_or("")
+    };
+    let host = host.trim();
     if host.is_empty() {
         None
     } else {
@@ -103,6 +110,20 @@ mod tests {
         assert_eq!(host_of(""), None);
     }
 
+    #[test]
+    fn ipv6_literals_are_distinct_hosts_not_the_bracket() {
+        assert_eq!(host_of("http://[::1]:8080/x"), Some("::1".to_string()));
+        assert_eq!(
+            host_of("http://[2001:db8::1]/x"),
+            Some("2001:db8::1".to_string())
+        );
+        assert_ne!(
+            host_of("http://[::1]/x"),
+            host_of("http://[::2]/y"),
+            "distinct IPv6 hosts must not collapse onto one pacer key"
+        );
+    }
+
     #[tokio::test]
     async fn zero_interval_paces_nothing() {
         let pacer = HostPacer::new(Duration::ZERO);

src/llm/cache.rs

@@ -10,12 +10,15 @@ use std::path::PathBuf;
 
 use crate::llm::prompt::Verdict;
 
-/// A stable hex key for the given inputs. Order-sensitive.
-pub fn cache_key(provider: &str, model: &str, check: &str, text: &str) -> String {
+/// A stable hex key for the given inputs. Order-sensitive. The base URL is
+/// part of the key: two different OpenAI-compatible endpoints can share a
+/// provider label and model string, and a verdict produced by one must not
+/// be served for the other.
+pub fn cache_key(provider: &str, base_url: &str, model: &str, check: &str, text: &str) -> String {
     const OFFSET: u64 = 0xcbf2_9ce4_8422_2325;
     const PRIME: u64 = 0x0000_0100_0000_01b3;
     let mut hash = OFFSET;
-    for field in [provider, model, check, text] {
+    for field in [provider, base_url, model, check, text] {
         for byte in field.as_bytes() {
             hash ^= u64::from(*byte);
             hash = hash.wrapping_mul(PRIME);
@@ -77,17 +80,28 @@ mod tests {
     #[test]
     fn key_is_deterministic_and_field_sensitive() {
         assert_eq!(
-            cache_key("openai", "m", "check", "text"),
-            cache_key("openai", "m", "check", "text")
+            cache_key("openai", "https://api.openai.com/v1", "m", "check", "text"),
+            cache_key("openai", "https://api.openai.com/v1", "m", "check", "text")
         );
         assert_ne!(
-            cache_key("openai", "m", "check", "text"),
-            cache_key("openai", "m", "check", "other")
+            cache_key("openai", "https://api.openai.com/v1", "m", "check", "text"),
+            cache_key("openai", "https://api.openai.com/v1", "m", "check", "other")
+        );
+        // Different endpoint, same provider/model: must not share a verdict.
+        assert_ne!(
+            cache_key("openai", "https://api.openai.com/v1", "m", "check", "text"),
+            cache_key(
+                "openai",
+                "https://internal.example/v1",
+                "m",
+                "check",
+                "text"
+            )
         );
         // Field-boundary sensitivity: concatenation must not collide.
         assert_ne!(
-            cache_key("openai", "m", "ab", "c"),
-            cache_key("openai", "m", "a", "bc")
+            cache_key("openai", "u", "m", "ab", "c"),
+            cache_key("openai", "u", "m", "a", "bc")
         );
     }