Two robustness/security bugs from a full-tree audit, each with a test.
- providermodeldiscovery: when a live /models probe succeeds (200) but its
model ids don't match the curated catalog, the merge was empty and the whole
live+curated list was discarded — the picker collapsed to the bare built-in
set with a misleading "no model ids" error. Now fall back to the curated
catalog instead of returning an empty list.
- redaction: Authorization / Proxy-Authorization headers carrying an opaque
token or a custom (non-standard) scheme were not redacted in free text (logs,
command/diff output, errors, session search), echoing live credentials back
into model context and rendered output. The scheme is now optional, so any
credential value is redacted to end-of-line; a recognized scheme name is
still kept for readability.
* redaction: catch compound secret keys + non-bearer auth schemes; stop scanner over-redaction
Hardens the secret-redaction last line of defense after an audit of
internal/redaction + internal/secrets.
1. Compound secret keys are now redacted. IsSensitiveKey was exact-match only, so
db_password, session_secret, stripe_secret_key, auth_token, csrf_token,
ssh_private_key, backup_api_key, etc. passed through in cleartext. A new
keyLooksSensitive applies conservative structural heuristics (secret /
password / passphrase / credential / apikey segments; a singular trailing
"_token"; api_key / private_key pairs). Critically, the agent's token-COUNT
fields (max_tokens, prompt_tokens, token_count) and ordinary *_key fields
(primary_key, public_key, cache_key) are deliberately NOT redacted.
2. Authorization headers with non-bearer/basic schemes were leaking. The header
pattern now also covers token, apikey, digest, negotiate, oauth, and
aws4-hmac-sha256.
3. The secrets scanner over-redacted ordinary text: its prefixed patterns (sk-,
gh*, AIza, AKIA, xox, github_pat, jwt) lacked the word boundaries that
internal/redaction already uses, so kebab-case words like
"task-management-and-coordination" matched as a fake key. Added \b anchors.
Design note: a free-text "key: value" colon pattern was prototyped but dropped --
it mangled "file.go:line: message" output (a filename's "secret" segment looked
like a sensitive key), the same over-redaction class fix#3 addresses. The
structured paths (RedactValue for maps/structs, plus assign/JSON/query forms in
RedactString) are covered; free-text colon-form remains out of scope.
Tests: compound-key + token-count-safety matrix, auth-scheme redaction, the
file:line guard, and scanner boundary positives/negatives.
* redaction: redact the full auth-header value for multi-part schemes
The broadened header pattern captured only the first token after the scheme, so
parameterized schemes (Digest "…, response=…", AWS4-HMAC-SHA256 "…, Signature=…")
left the actual secret param visible. Redact the entire value to end of line; the
scheme name is kept. Adds tests with realistic Digest + AWS SigV4 headers.
Addresses review feedback.
* audit: fix redaction sibling-DAG false cycle, hoist URL regex, widen OpenAI key pattern
- redaction: seen-set now tracks only the current DFS path (delete pointer
after recursing) so a shared reference reached via a sibling branch is no
longer reported as CircularReference; true cycles still detected. (audit
redaction.go:222)
- redaction: redactURLPasswords compiles its regexp once at package scope
instead of per call. (audit redaction.go:362)
- secrets: openai_key body allows - and _ so sk-proj-/sk-svcacct- keys match,
not just legacy sk-<alnum>. (audit scanner.go:35)
* audit: rune-safe cron run-error truncation and commit-subject length
- cronTruncate now cuts on a UTF-8 rune boundary (via cutRuneBoundary) so a
persisted cron run-error excerpt can't end in a split rune. (audit cron_run.go:179)
- zerogit ValidateMessage counts runes, not bytes, so a valid non-ASCII commit
subject under 72 chars is no longer rejected. (audit zerogit.go:207)
* audit: report auth-header-only profiles as having a credential
ProviderSnapshot.APIKeySet was true only for a non-empty APIKey, so a profile
authenticated solely via AuthHeaderValue rendered as 'not set' in the command
center. Treat either credential as set. (audit contracts.go:166)
* audit: address review on #174 (credential-presence coverage + consistency)
- Add ProviderSnapshotFromProfile test covering APIKeySet for api-key-only,
auth-header-only, both, and neither (the AuthHeaderValue path CodeRabbit
flagged as untested).
- doctor.go: report the provider 'apiKey' diagnostic as set when EITHER APIKey
or AuthHeaderValue is present, matching ProviderSnapshot.APIKeySet — closing
the same auth-header-only gap at the other credential-check site.
- scanner_test: also assert the [REDACTED:openai_key] placeholder is present
(matches the pattern used by the existing redaction test).
* audit: trim credentials + surface doctor credential presence (#174 review)
- ProviderSnapshot test: add whitespace-only api-key / auth-header cases so the
TrimSpace behavior stays covered.
- doctor.go: trim both APIKey and AuthHeaderValue before the presence check
(matches ProviderSnapshot.APIKeySet) AND move the indicator off the sensitive
'apiKey' detail key — check() runs RedactValue, so 'apiKey' was always scrubbed
to [REDACTED], leaving the set/not-set indicator invisible. Now reported under
'credentialConfigured' so it actually surfaces; added a doctor table test.
No consumer read the old detail key.
* audit: honor AuthHeaderValue in model discovery + base-URL redaction (#174 review)
Two credential-presence consistency gaps flagged on #174:
- providermodeldiscovery: the /models probe gated on (and sent Authorization
from) profile.APIKey only, so an auth-header-only profile was skipped/sent
unauthenticated. Gate via discoveryHasCredential (APIKey OR AuthHeaderValue)
and build the request with providerio.ApplyAuthHeaders, matching how the live
providers authenticate (honors AuthHeaderValue, AuthHeader, AuthScheme,
CustomHeaders). APIKey-only behavior is unchanged.
- zerocommands.redactProviderBaseURL now also redacts AuthHeaderValue if it
leaks into the base URL (variadic secrets, empties dropped).
Adds regression tests for auth-header-only discovery, discoveryHasCredential,
and auth-header redaction in the base URL.
* audit: doctor credential-presence test parity with contracts_test (#174 review)
Add the 'both' and 'whitespace auth header only' cases so
TestProviderConfigCheckCredentialPresence matches the six cases in
contracts_test.go's TestProviderSnapshotAPIKeySetCountsAuthHeaderValue.
KRATOS