fix(diagnostics): keep credentials and private hosts out of diagnostic and config output

docs-site/src/content/docs/reference/cli/agents.md

@@ -397,6 +397,10 @@ An observed identity or digest describes those files during this observation. It
 Inspect and safely modify validated OpenCodex configuration. `show` and `get` mask secrets. Import
 validates before writing and requires `--yes`.
 
+Display and mutation output strip credentials from proxy URLs while retaining the host and port.
+`direct` and credential-free proxy values stay readable. `export` preserves credentials so the
+backup can restore the configuration; store exported files as secrets.
+
 ### Usage from a connected client
 
 `ocx usage` reads the connected hub with this client's enrolled data key. Human output identifies the hub source and client-key scope; `--json` returns the same scoped data. Range, surface, provider/model filters and custom `--since`/`--until` bounds remain available. Account breakdowns and other clients' records are not shared. An old or unavailable hub produces an explicit error instead of substituting local usage; upgrade the hub if it does not support this read.

scripts/test-layout/layout.json

@@ -1175,6 +1175,7 @@
     "provider-outbound-private-network.test.ts": "providers",
     "provider-outbound.test.ts": "providers",
     "provider-payload.test.ts": "gui",
+    "provider-quota-label-sanitize.test.ts": "providers",
     "provider-quota-observed-marker.test.ts": "providers",
     "provider-quota.test.ts": "providers",
     "provider-registry-parity.test.ts": "providers",

src/cli/config-command.ts

@@ -5,6 +5,7 @@ import { VISION_REASONING_EFFORTS, isVisionReasoningEffort } from "../reasoning-
 import type { OcxConfig } from "../types";
 import { normalizeVisionReasoningForModel } from "../vision/reasoning";
 import type { ServiceApiTokenState } from "../lib/service-secrets";
+import { redactUrlForLog } from "../lib/redact";
 import { CliUsageError, printData, rejectArgs, runCliAction, takeFlag } from "./runtime-api";
 
 const USAGE = `Usage:
@@ -17,12 +18,13 @@ const USAGE = `Usage:
   ocx config import <path|-> --yes [--json]`;
 
 /**
- * Keys whose VALUE is a credential and must never be printed or exported.
+ * Keys whose VALUE is a credential and must never be printed by display commands.
+ * `config export` writes the raw config so an export can restore credentials; it does
+ * not call `redact`.
  *
- * `webhookUrl` is here because for Slack and Discord the URL itself is the authorization:
- * anyone holding it can post to the channel. It looks like configuration rather than a secret,
- * which is exactly why it needs to be named explicitly — none of the other patterns match it,
- * so `ocx config show` printed it and `config export` wrote it to disk in the clear.
+ * URL-valued credentials must be named explicitly: `webhookUrl` matches none of the
+ * other patterns, and a proxy URL's userinfo is handled by the `proxy` branch in
+ * `redact` rather than by masking the whole value.
  */
 const SECRET_KEYS = /^(apiKey|key|accessToken|refreshToken|idToken|token|password|clientSecret|webhookUrl)$/i;
 const BLOCKED_SEGMENTS = new Set(["__proto__", "prototype", "constructor"]);
@@ -97,6 +99,19 @@ async function readRemoteHubConfigNote(config: OcxConfig): Promise<ReturnType<ty
 }
 
 function redact(value: unknown, key = ""): unknown {
+  if (key === "proxy" && typeof value === "string") {
+    // "direct" and credential-less proxy URLs carry no secret and stay readable; only a
+    // URL with userinfo is masked, and then only its credentials — host and port stay
+    // visible so the output still says WHERE traffic goes. A non-URL value that is not
+    // "direct" cannot be proven credential-free, so it is masked whole.
+    if (!value || value === "direct") return value;
+    try {
+      const parsed = new URL(value);
+      return parsed.username || parsed.password ? redactUrlForLog(value) : value;
+    } catch {
+      return "********";
+    }
+  }
   if (SECRET_KEYS.test(key) && typeof value === "string") return value ? "********" : value;
   // `client.priorCatalog` is the base64 catalog snapshot connect took before overwriting the
   // local one — up to 64 MB of it (src/config.ts). Printed in full it buried `runtimeRole` and
@@ -211,7 +226,7 @@ export async function handleConfigCommand(argv: string[]): Promise<number> {
       const path = args.shift();
       if (!path) throw new CliUsageError("config path is required", USAGE);
       rejectArgs(args, USAGE);
-      const value = redact(getPath(readConfigDiagnostics().config, path), path.split(".").at(-1));
+      const value = redact(getPath(readConfigDiagnostics().config, path), pathSegments(path).at(-1));
       if (wantsJson || typeof value === "object") console.log(JSON.stringify(value, null, 2));
       else console.log(String(value));
       return;
@@ -257,7 +272,7 @@ export async function handleConfigCommand(argv: string[]): Promise<number> {
           ? "config changed while applying this update; retry"
           : `config is ${outcome.reason}`);
       }
-      printData({ ok: true, path, value: redact(savedValue, path.split(".").at(-1)) }, wantsJson,
+      printData({ ok: true, path, value: redact(savedValue, pathSegments(path).at(-1)) }, wantsJson,
         [`${action === "unset" ? "Unset" : "Set"} ${path}.`]);
       return;
     }

src/config/load-degrade.ts

@@ -420,10 +420,9 @@ export function degradedCredentialGroupsWarning(rawParsed: unknown): string | nu
   if (!pool || pool.credentialGroups === undefined) return null;
   const parsed = credentialGroupsSchema.safeParse(pool.credentialGroups);
   if (parsed.success) return null;
-  // Every issue message is redacted before it is joined. The custom messages embed the
-  // offending member through `JSON.stringify`, so a malformed credential string that
-  // happens to carry secret material would otherwise be printed verbatim at config load
-  // — a config file is exactly where a pasted token ends up in the wrong field.
+  // Every issue message is redacted before it is joined. The custom messages now name
+  // group/member positions instead of the offending strings; the redaction stays as a
+  // second layer for any schema default message that still embeds a value.
   const details = parsed.error.issues.map(issue => redactSecretString(issue.message)).join("; ");
   return `pool.credentialGroups is invalid (${details}) — declared quota grouping is disabled; other pool settings were preserved`;
 }

src/lab/artifacts/sanitize.ts

@@ -156,8 +156,9 @@ const HOSTNAME_RE = /(?<![\w.-])(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.){1,8}(
 // `ETIMEDOUT after 30 seconds` into `ETIMEDOUT [host] 30 seconds`: a redaction
 // that destroys the diagnostic and hides nothing.
 // Markers differ in confidence, and treating them alike cost accuracy both
-// ways. STRONG markers are resolver/socket errors and `host=`: whatever
-// follows is a host by construction, so a bare `redis` or `localhost` counts.
+// ways. STRONG markers are resolver/socket errors and `host=`: the argument
+// position is a host by construction, so a bare `redis` or `localhost` counts
+// there while the prose that can follow a marker survives.
 // WEAK markers appear in ordinary prose (`upstream provider.metric.p95
 // exceeded`), so they only redact a candidate that is already host-shaped.
 const STRONG_HOST_CONTEXT_RE =
@@ -166,11 +167,12 @@ const WEAK_HOST_CONTEXT_RE =
   /\b(?:upstream)["']?\s*[\s=:]\s*["']?([A-Za-z0-9_.-]{1,255})/gi;
 
 /**
- * A dotted run of plain alphabetic words with no digits or hyphens in any
- * label — `provider.metric.p95` reads as a namespace, not a host. Under a
- * WEAK marker this is left alone; under a STRONG one the marker decides.
+ * A dotted run of plain alphabetic words, or the conventional
+ * `<name>.metric.p<digits>` shape, reads as a namespace rather than a host.
+ * Other digit-suffixed labels are host-shaped and must not receive this weak
+ * marker exemption. Under a STRONG marker the marker always decides.
  */
-const DOTTED_NAMESPACE_RE = /^[a-z]+(?:\.[a-z]+)*\.[a-z]+[0-9]*$/i;
+const DOTTED_NAMESPACE_RE = /^(?:[a-z]+(?:\.[a-z]+)+|[a-z]+(?:\.[a-z]+)*\.metric\.p[0-9]+)$/i;
 
 /**
  * Does this token look like a host rather than an English word?
@@ -184,8 +186,10 @@ const DOTTED_NAMESPACE_RE = /^[a-z]+(?:\.[a-z]+)*\.[a-z]+[0-9]*$/i;
  * A stopword list would repeat the delimiter-enumeration mistake, so the
  * candidate is validated instead. A token qualifies when it carries host
  * punctuation (dot, hyphen, underscore, digit) or is a reserved name; a bare
- * English word does not — EXCEPT directly after a resolver marker, where the
- * argument is a name by construction and `ENOTFOUND redis` must still redact.
+ * English word does not — EXCEPT in a grammar position proven to hold the
+ * destination of a resolver or socket-state marker, where the argument is a
+ * name by construction and `ENOTFOUND redis` and `ECONNREFUSED redis` must
+ * still redact.
  */
 const RESERVED_HOST_NAMES = new Set(["localhost", "broadcasthost"]);
 const PROSE_AFTER_MARKER = new Set([
@@ -198,8 +202,8 @@ function isHostCandidate(value: string, bareWordAllowed = false): boolean {
   if (/[.\-_0-9]/.test(value)) {
     return AMBIGUOUS_HOST_RE.test(value) || CONTEXTUAL_HOST_TOKEN_RE.test(value);
   }
-  // A bare word: only a resolver marker makes it a host, and only when it is
-  // not one of the connective words those messages actually use.
+  // A bare word: only a destination-bearing marker makes it a host, and only
+  // when it is not one of the connective words those messages actually use.
   return bareWordAllowed && !PROSE_AFTER_MARKER.has(lower);
 }
 
@@ -499,8 +503,13 @@ function scrubString(value: string): string {
   s = s.replace(STRONG_HOST_CONTEXT_RE, (m, tail: string) => {
     // Scan the few tokens after the marker for the first host-shaped one:
     // Go writes `dial tcp: lookup <host>: no such host`, so the destination
-    // is not always adjacent to the marker. A resolver marker also licenses a
-    // bare name (`ENOTFOUND redis`), which a socket-state marker does not.
+    // is not always adjacent to the marker. The destination sits inside
+    // `tail`, so the rewrite is spliced at an offset in it — replacing
+    // against the whole match lets a marker word that repeats the
+    // destination (`connect to connect failed`) take the `[host]` instead.
+    const head = m.slice(0, m.length - tail.length);
+    const redact = (at: number, token: string) =>
+      head + tail.slice(0, at) + "[host]" + tail.slice(at + token.length);
     // A name paired with a port is a destination whatever else is true:
     // `dial tcp redis:6379` needs no other evidence. Both notations count —
     // adjacent `host:443` and spelled-out `gateway on port 443` — because the
@@ -509,13 +518,45 @@ function scrubString(value: string): string {
       tail.match(/(?<![\w.-])([A-Za-z0-9_.-]{1,255}):\d{1,5}(?![\w.])/) ??
       tail.match(/(?<![\w.-])([A-Za-z0-9_.-]{1,255})\s+(?:on\s+)?port\s+\d{1,5}\b/i);
     if (ported?.[1] && !PROSE_AFTER_MARKER.has(ported[1].toLowerCase())) {
-      return m.replace(ported[1], "[host]");
+      return redact(ported.index ?? 0, ported[1]);
     }
-    // Otherwise only a resolver marker licenses a bare name. Natural-language
-    // `connect to` does not: `Unable to connect to your account` is prose.
-    const resolver = /ENOTFOUND|EAI_AGAIN|lookup|host/i.test(m);
-    for (const token of tail.split(/[\s:]+/)) {
-      if (token && isHostCandidate(token, resolver)) return m.replace(token, "[host]");
+    // A failure immediately following an unported connect-to target supplies
+    // the missing network context without making ordinary connective prose
+    // (`connect to your account`) host-bearing.
+    const failedConnectTarget = /\bconnect(?:ing)?\s+to\b/i.test(m)
+      ? tail.match(/^([A-Za-z][A-Za-z0-9]{0,254})\s+(?:failed|refused|unreachable|reset|timed\s+out)\b/i)
+      : null;
+    if (failedConnectTarget?.[1]) {
+      return redact(failedConnectTarget.index ?? 0, failedConnectTarget[1]);
+    }
+    // A bare destination name is licensed only where the grammar proves the
+    // position is the destination: directly after a resolver or explicit host
+    // marker, as a socket marker's sole argument
+    // (`ECONNREFUSED redis`, `dial tcp redis`) or as the argument of `lookup`
+    // (`dial tcp: lookup redis`). Past an open-ended connective chain nothing
+    // proves the next word is a destination — `ETIMEDOUT while waiting for
+    // response` is prose. Natural-language `connect to` licenses nothing:
+    // `Unable to connect to your account` is prose.
+    const destinationContext =
+      /ENOTFOUND|EAI_AGAIN|ECONNREFUSED|ETIMEDOUT|EHOSTUNREACH|dial\s+(?:tcp|udp)|lookup|\bhost\b/i.test(m);
+    // Direct resolver and explicit host markers name their first argument even
+    // when explanatory prose follows; socket-state prose remains ambiguous.
+    const directHostArgument = /^(?:ENOTFOUND|EAI_AGAIN|host)\b/i.test(head);
+    const tokens = tail.split(/[\s:]+/).filter(Boolean);
+    let cursor = 0;
+    for (let i = 0; i < tokens.length; i += 1) {
+      const token = tokens[i]!;
+      const at = tail.indexOf(token, cursor);
+      cursor = at + token.length;
+      // A sentence-final period travels with the token (`ECONNREFUSED redis.`):
+      // classify the name without it, and leave it outside the mask so the
+      // message still reads as a sentence.
+      const core = token.replace(/\.+$/, "");
+      if (!core) continue;
+      if (isHostCandidate(core)) return redact(at, core);
+      const bareLicensed =
+        destinationContext && (tokens.length === 1 || tokens[i - 1]?.toLowerCase() === "lookup" || (i === 0 && directHostArgument));
+      if (bareLicensed && isHostCandidate(core, true)) return redact(at, core);
     }
     return m;
   });
@@ -583,4 +624,4 @@ export function sanitizeDiagnostic(value: unknown): string {
 
 export function sanitizedJsonBytes(value: unknown): Uint8Array {
   return new TextEncoder().encode(jcsStringify(scrubValue(value, 0)));
-}
+}

src/providers/quota/vendor-probes-oauth.ts

@@ -224,20 +224,27 @@ function parseClaudeBucket(value: unknown): { percent?: number; resetAt?: number
   return { percent, resetAt };
 }
 
+const TERMINAL_CONTROL_CHARACTERS = /[\u0000-\u001f\u007f-\u009f]/gu;
+
 function parseClaudeLimit(value: unknown): { label: string; percent: number; resetAt?: number } | null {
   const rec = asRecord(value);
   if (!rec) return null;
   const percent = normalizePercent(rec.percent);
   if (percent === undefined) return null;
   const scope = asRecord(rec.scope);
   const model = asRecord(scope?.model);
-  const rawLabel = String(model?.display_name ?? "").trim();
+  const rawLabel = String(model?.display_name ?? "")
+    .replace(TERMINAL_CONTROL_CHARACTERS, "")
+    .trim();
   if (!rawLabel) return null;
   const lowerLabel = rawLabel.toLowerCase();
   const label = lowerLabel.includes("fable") ? "Fable"
     : lowerLabel.includes("opus") ? "Opus"
       : lowerLabel.includes("sonnet") ? "Sonnet"
-        : rawLabel;
+        : null;
+  // An unrecognized display_name is never published as a quota label: stripping
+  // control characters still leaves attacker-chosen residue on the quota line.
+  if (label === null) return null;
   const resetAt = normalizeResetAt(rec.resets_at);
   return { label, percent, ...(resetAt !== undefined ? { resetAt } : {}) };
 }

src/routing/identity-domains.ts

@@ -259,38 +259,45 @@ function memberMatches(member: string, ref: CredentialDomainRef): boolean {
 }
 
 /**
- * Every way a declared grouping can be ambiguous, as operator-readable messages. The
- * config write path rejects on any of these and the load path drops the list, so an
- * ambiguous declaration is reported rather than resolved by whichever group came first.
+ * Every way a declared grouping can be ambiguous, reported by position only. Messages
+ * name group and member indexes, never the operator-supplied strings — a malformed
+ * credential pasted into this list would otherwise be printed verbatim into shared
+ * logs. The config write path rejects on any of these and the load path drops the
+ * list, so an ambiguous declaration is reported rather than resolved by whichever
+ * group came first.
  */
 export function credentialGroupIssues(groups: readonly DeclaredCredentialGroup[]): string[] {
   const issues: string[] = [];
-  const seenIds = new Set<string>();
-  const owner = new Map<string, string>();
-  for (const group of groups) {
+  const seenIds = new Map<string, number>();
+  const owner = new Map<string, { groupIndex: number; memberIndex: number }>();
+  for (const [groupIndex, group] of groups.entries()) {
     // A duplicate id is not cosmetic: both groups key to `declared:<id>`, so the second
     // group's members join the first group's quota domain without anyone saying so.
-    if (seenIds.has(group.id)) issues.push(`duplicate group id ${JSON.stringify(group.id)}`);
-    seenIds.add(group.id);
+    const firstGroupIndex = seenIds.get(group.id);
+    if (firstGroupIndex !== undefined) {
+      issues.push(`duplicate group id at group index ${groupIndex} (first declared at group index ${firstGroupIndex})`);
+    } else {
+      seenIds.set(group.id, groupIndex);
+    }
     if (group.credentials.length === 0) {
-      issues.push(`group ${JSON.stringify(group.id)} lists no credentials`);
+      issues.push(`group at index ${groupIndex} lists no credentials`);
     }
-    for (const member of group.credentials) {
+    for (const [memberIndex, member] of group.credentials.entries()) {
       if (splitMember(member) === undefined) {
         issues.push(
-          `group ${JSON.stringify(group.id)} member ${JSON.stringify(member)} must be provider-qualified as "<provider>:<credential-id>"`,
+          `credential at member index ${memberIndex} in group index ${groupIndex} must be provider-qualified as "<provider>:<credential-id>"`,
         );
         continue;
       }
       const existing = owner.get(canonicalMember(member));
-      if (existing === group.id) {
-        issues.push(`credential ${JSON.stringify(member)} is listed twice in group ${JSON.stringify(group.id)}`);
+      if (existing?.groupIndex === groupIndex) {
+        issues.push(`credential at member index ${memberIndex} is listed twice in group index ${groupIndex}`);
       } else if (existing !== undefined) {
         issues.push(
-          `credential ${JSON.stringify(member)} is declared in more than one group (${existing}, ${group.id})`,
+          `credential at member index ${memberIndex} in group index ${groupIndex} is declared in more than one group (first declared at group index ${existing.groupIndex}, member index ${existing.memberIndex})`,
         );
       } else {
-        owner.set(canonicalMember(member), group.id);
+        owner.set(canonicalMember(member), { groupIndex, memberIndex });
       }
     }
   }