flan
26d5eda1c7
Build and Package / Build Binaries (push) Has been cancelled
An independent audit of this fork found the headline claim -- 'the API key never
reaches a log line' -- was broken on a path I had not covered.
formatAPIErrorWithContext serialized the request params into the error string, and
for auth.login_ex those params ARE the plaintext key. Not debug-gated, not
redacted. Any middleware error frame on authentication (bad key, or a reconnect
that re-authenticates and fails mid-session) put the key into log.Fatalf at startup
AND, through CallTool's error return, into the model's context and the transcript.
My redaction test only covered the debug request-log frame, which is precisely why
this survived.
Worse, the deprecated auth.login_with_api_key passes the key as a bare positional
param -- a naked string with no key name -- so key-based redaction was structurally
incapable of masking it. redactParamsForError now masks every scalar param of any
auth.* method outright, leaving maps to key-based redaction so username/mechanism
still show up in the error.
Redaction gaps also closed: env vars come back as [{name:DB_PASSWORD,value:
...}], where the secret sits under the generic key value and no key rule could
see it; bindpw/keytab/bare-key were absent from the hints (upstream's
maskCredentials only masks those at the top level, so a nested one bypassed both);
and the float64 round-trip silently corrupted 64-bit integers like ZFS guids.
-insecure was a no-op: verification was always off while the flag claimed to be
what turned it off. That undercuts the ws:// rejection entirely -- refusing
plaintext to protect the key is hollow if the wss:// connection trusts any cert.
Verification is now on by default; -insecure genuinely disables it and warns.
TrueNAS is self-signed, so -insecure is required against a stock box -- but as an
explicit choice, not a silent default.
Remaining limit is documented, not hidden: redaction is key-name-based, so a secret
inside an opaque string blob (a custom app's compose YAML) is not caught. A test
pins that behaviour so it can't be mistaken for safety.
Read-only gate audited clean: gate before dispatch, fail-closed for unknown tools,
no mutating middleware method reachable from an allowlisted tool.