Repository navigation
security(llc): globally-unique agent_id makes cross-company slug existence probeable #15812
Description
Parent: #13935 · raised by review on #15804
The oracle, and why it is not closable at the route
AgentOrgNode.agent_id is globally unique — models/agent_org.py:59, unique=True, no company scoping and no composite constraint. So PUT /agents/{agent_id}/org cannot avoid distinguishing two cases:
| Caller's company | Slug state | Response |
|---|---|---|
| any | slug free | node created, 200 |
| any | slug taken in another company | 404 (after #15804's tenant scoping) |
A caller holding admin.reporting_line.write can therefore probe whether an agent_id exists somewhere they cannot see.
#15804 tightened this from 500-vs-200 to 404-vs-200, which fixed the shape of the error and not the distinguishability — the two outcomes still differ, so the oracle survived the fix. That is worth stating plainly rather than leaving the earlier commit message to imply the leak was closed.
Why response shaping cannot fix it
Every option collapses:
- Return 404 for both → the endpoint stops creating agents, which is its purpose.
- Return 200 for both → a lie, and the subsequent read fails.
- Return 409 for a taken slug → still "taken vs free", the same bit.
The distinguishability follows from a global namespace, not from the handler. It is the same class as username enumeration at signup: any system where names are globally unique and creation is permitted will tell you whether a name is taken.
What the leak is worth
Narrower than it first reads. It reveals "this slug exists somewhere", not which company holds it, nor anything about that company. agent_id is an operator-chosen slug (str(uuid4()) in the current seeding paths), so the namespace is not guessable in practice, and the caller must already hold admin.reporting_line.write.
That is an argument for prioritising it, not for dismissing it: a low-value oracle is still an oracle, and it becomes materially worse if agent slugs ever become human-readable (acme-support-bot), because guessable names turn a per-probe question into an enumeration.
The real decision
Should agent_id be unique per company rather than globally?
Blast radius is the reason this is its own issue rather than a fix on #15804: the slug is a keyspace, not just a name. Budgets, heartbeat runs and controls key on it (llc/api/companies.py reads budgets.get(row.agent_id), runs.get(row.agent_id)), and reports_to referenced it before #15763 moved reporting lines onto the UUID assignment keyspace (#10032). Making it company-scoped means every one of those lookups becomes (company_id, agent_id).
Acceptance
- A decision is recorded: scope
agent_idper company, or accept the oracle with the reasoning above - If scoped: every consumer keying on the slug is migrated, with no lookup left keyed on the slug alone
- If accepted: the accepted risk is recorded where the constraint is declared, so the next reviewer finds the decision rather than re-deriving it
- Either way,
PUT /agents/{agent_id}/orgstops being the only place this is documented
Activity
fix(llc): enforce the company boundary on the CEO designation (#15770)
mrveiss commentedon Sep 6, 2026
Decision recorded: scope per company, staged
A second instance turned up while measuring the blast radius, and it is the worse of the two.
agent_idcompany_idagent_org_nodesmodels/agent_org.py:59):65)llc_agent_budgetsllc/models/budget.py:32):31)llc_heartbeat_runsllc/models/heartbeat_run.py:40)llc_agent_api_keysllc/models/api_key.py:27)llc_agent_budgetscarries the scoping column already, non-nullable, and does not use it. So thesame "is this slug taken" bit is answerable there, and closing it costs one constraint change with no
data migration at all. It is a strictly cheaper fix than the one this issue was opened for.
And
agent_org_nodescannot simply take a composite constraint.company_idis nullable bydesign — migration
20260523_037states it outright: "The column is nullable so existing rows areunaffected; operators must set it". A
UniqueConstraint(company_id, agent_id)does not constrainrows where
company_id IS NULL, becauseNULL != NULLunder SQL unique semantics. Adding theconstraint there without a backfill would look like a fix, pass its migration, and leave every
unscoped row free to duplicate — a guard that reports clean having constrained nothing.
The plan
Wave 1 —
llc_agent_budgets, this PR. Swapunique=TrueforUniqueConstraint(company_id, agent_id)and migrate every lookup keyed on the slug alone. Nobackfill, no nullable column, no expand/contract.
Wave 2 —
agent_org_nodes, its own issue. Backfillcompany_id, make itNOT NULL, thenscope the constraint and migrate the slug-keyed consumers. Expand/contract, and it needs to be
reviewed as a data migration rather than as a constraint change.
Why staged rather than one PR: the two have nothing in common but the symptom. One is a
zero-risk constraint swap; the other rewrites rows an operator may never have set
company_idon.Landing them together gives them a shared fate, so a wrong backfill takes the safe fix down with it,
and it puts a reviewer's attention on a data migration and a one-line constraint in the same pass.
Wave 2 is filed and linked below. This issue stays open until both land — the decision it asks for is
recorded here, but the oracle is only half closed by Wave 1.
style(llc): wrap two ingest_cost_event call sites under the line limit (
fix(llc): update the two direct _validate_budget test calls to the sc…
fix(llc): scope the watchdog pause and the in-process run, and clear …
fix(llc): build the scoped pause through the ORM so the UUID actually…
feat(llc): give every company a CEO, including the ones that already …
feat(llc): give every company a CEO, including the ones that already …
chore: claim worktree for #15812
fix(llc): renumber to 089 behind the landed 088, and await the task t…
fix(llc): bind the company as a UUID in both org-node joins, and fix …
fix(llc): keep the budget hard stop working on un-backfilled org nodes (
fix(security): scope the agent budget slug to its company (#15812) (#…
mrveiss commentedon Sep 6, 2026
Closed by #15864, merged as
fb9b3f2c0. Every criterion verified against merged base (git show origin/Dev_new_gui:<path>), not against the branch.llc/models/budget.py:29—UniqueConstraint("company_id", "agent_id", name="uq_llc_agent_budgets_company_id_agent_id"), with migration20260906_089_llc_agent_budgets_company_scope.pyin baseLLCAgentBudget.agent_id ==inautobot-backend/llc/outside tests was checked for acompany_idpredicate within its query: 0 unscoped.services/budget.py:48-51is the canonical selector and carries both;api/budget.pyalone carries 26company_idreferencesPUT /agents/{agent_id}/orgstops being the only place documentedllc/models/budget.py:33-34documents it at the column: "Per-company slug. NOT globally unique (#15812): global uniqueness made 'is this slug taken?' answerable across the tenant boundary"The second criterion is the one worth showing the method for. "No lookup left keyed on the slug alone" is a claim about absence, and a grep for the scoped form would confirm it while telling you nothing about the unscoped ones. So the check enumerated every occurrence of the slug predicate and asserted a
company_idpredicate accompanies each — a positive assertion over the whole population rather than a negative one over a sample.What is deliberately not covered here
agent_org_nodes.agent_idremains globally unique. The two tables now disagree about what anagent_ididentifies, and that is #15858's scope, not this one — this issue is the budget-slug oracle specifically. Recording it so the difference reads as a known boundary rather than an oversight.Review note
The blocking finding on #15864 came from a second session and was correct: the added
AgentOrgNode.company_id == company_uuidpredicate matched nothing forNULLrows on that un-backfilled column, so the budget hard stop silently disabled itself — a zero-rowUPDATEraises nothing and the caller swallowed the result. A security control reporting success having done nothing. Fixed with anis_(None)arm and arowcount == 0warning, plus tests against a real SQLite engine; the existing watchdog tests all mocked the session factory, which is why nothing caught a SQL NULL-semantics bug.feat(llc): the three remaining agent routes do their work (#15905)
feat(llc): the three remaining agent routes do their work (#15905) (#…