Repository navigation
security(connectors): REST connector path still mints a shared "system" owner #13702
Description
Activity
- added a commit that references this issue
on Aug 8, 2026 Write path fixed by #13725, merged as
d135f39f2.Evidence
python-suite: 12 / 12 shards SUCCESS knowledge/connectors/ + api connector tests: 387 passedcreate_connectornow derives the credential owner from the authenticated caller (user_id -> sub -> username) instead of minting the shared literal"system", and refuses with 401 when no identity resolves. Every new test was run against the reinstatedor "system"to confirm it fails there:4 failedagainst the bug,16 passedrestored.Two pre-existing tests asserted the bug —
assert_awaited_once_with("sec-oauth-123", "system"). They now assert the caller's real identity.Severity, corrected
My original framing said "two users pass each other's ownership check". The router carries
dependencies=[Depends(check_admin_permission)], so it takes two admins, not two arbitrary users. Still a real isolation gap — connector credentials are per-owner and admins are distinct principals — but narrower than filed.Why this stays open
The three read sites (
:249,:598,:629) deliberately keepcfg.owner_id or "system". Rows created before this fix already carry that owner; dropping the fallback would orphan them rather than protect anything. They are annotated in place.Remaining: decide and execute the migration for existing
"system"-owned rows — migrate them to a real owner, or keep"system"as an explicit documented sentinel with its own access rule. Until then this is half-delivered, so it does not close.Session ending (weekly token limit, owner call to drop concurrency). Not started -- no branch checks, no code reading done on this issue yet. Handing off as-is so the next session doesn't assume any progress here.
Context that may help: this issue was in a batch of 15 open security issues with unlanded branch work, dispatched by autobot-ai-df (queue coordinator) as part of a repo-wide stale-branch audit. Before touching any assigned branch, verify supersession first (compare each touched file's blob at branch tip against origin/main -- several branches in this same batch turned out to be stale pre-merge duplicates of already-merged PRs). Also check autobot-ai-df's trap.json scan (revival hazard: a branch whose removed lines cite an issue number lower than a later fix to the same file) before reviving anything -- this issue may or may not be on it, worth checking fresh.
The
or "system"shared-owner fallback removed from the OAuth start path in #13628 is still present on the REST connector path, and the same reasoning applies verbatim: it mints a shared literal owner, so every credential created that way is mutually readable by the others.Sites
autobot-backend/api/knowledge_connectors.py:431—owner_id = getattr(request, "owner_id", None) or "system"(the write; this is the one that creates the shared owner):249,:598,:629—cfg.owner_id or "system"Because
_require_owner(#13628) compares the storedcreated_byagainst the caller'sowner_id, two different users who both land on"system"pass each other's ownership check. The guard is working exactly as designed; the identity handed to it is the problem.Fix
Same shape as #13628's OAuth fix: resolve the caller's identity as
user_id -> sub -> username(the codebase's canonical order —api/voice.py:73,api/agent_terminal.py:614) and refuse only when none is present. Do not requireuser_idalone:get_current_userdoes not guarantee it — the internal-API-key path returns{"username": "service:slm", ...}, the dev-header path returns no id, and SLM-minted JWTs carrysub.The read sites need deciding separately: existing rows already store
"system", so simply removing the fallback there would orphan them. Either migrate those rows to a real owner or keep"system"as an explicit, documented sentinel with its own access rule.Acceptance criteria
"system"from a missing identitysub-only JWT callers all still succeed"system"-owned rows (migrate vs documented sentinel)Found during review of #13700 (#13628).