PR State:
merged
PR:
#85
Commit:
3d5894fc16ad488c2a9bc35bdef0ac946995cdf2
[FIX] viin_ai: has_api_key compute must not warm shared api_key cache (co-install #393211)
_compute_has_api_key read presence via bool(r.sudo().api_key). That ORM sudo
attribute read populates the transaction-wide cache for the group_system
api_key field; the cache is shared across sudo/non-sudo envs and the field
groups ACL is only enforced on a cache MISS, so a later low-privilege
record.api_key attribute access returned the cached secret WITHOUT the ACL
check - defeating the very ACL the docstring promised to preserve.
The WI-1 usable-provider resolver (_is_usable -> has_api_key /
_resolve_active_default) calls this in the provider-resolution hot path that
runs just before a Brain query-embedding call. Decisive base-vs-head repro
(Runbot subbuild #393211, co-install viin_ai_brain -> viin-brand-snailmail):
on HEAD a low-privilege brain semantic search (test_semantic_search_ilike_
fallback_*, uid 12/13) read the key from the warmed cache and fired a REAL
external embedding call, blocked by the test egress guard ("External requests
verboten"); the error-logging path then hit a rolled-back user
(MissingError -> viin_api_request_logger "Failed to create API request log"
ERROR + provider "could not write error usage.log" WARNING -> Runbot red).
On BASE (ef0d57fc) the api_key field ACL correctly denied the read and forced
the ILIKE fallback, no external call, no error. Tests PASS on both series
(0 failed/113); the regression is LOGGED-ERROR-only - Runbot reds on the
ERROR line, the assertion never fails.
Root cause is in viin_ai (this PR's own module), not viin_ai_brain (out of
scope) nor viin_api_request_logger (a tvtmaaddons dependency that merely
amplified the exposed call). Fix: compute presence straight from the row via
SQL (flush first so an uncommitted api_key write is reflected; NewId records
keep the plain cache read), so the safe boolean never pulls the secret into
the shared cache. Behaviour-preserving: has_api_key value unchanged, and the
low-privilege Brain search degrades to ILIKE exactly as before this PR.
Regression guard: TestProviderApiKeyAclCacheLeak asserts the deterministic
invariant (api_key never enters the shared cache via has_api_key / _is_usable)
- RED on the old bool(sudo().api_key) body (2 failed), GREEN on the fix. The
downstream attribute-access bypass itself is prefetch-order sensitive so the
cache-state invariant is the reliable oracle.
Verified: HEAD-with-fix co-install re-run -> verboten/MissingError/
Failed-to-create-API-log all 0 (was 2/4/2), api_key AccessDenied restored,
0 failed/113; full viin_ai (398) + viin_ai_rag (53) suites 0 failed.
Branch:
17.0