fix(security): enforce agent pool resource boundaries (IDOR-only) - #831
Conversation
|
Warning Review limit reachedNext included review available in 7 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| def _agent_in_pool(self, agent_pool_id: str, worker_agent_id: str) -> ModelAgent: | ||
| """Resolve an agent only through the pool boundary it can belong to. | ||
|
|
||
| The current persistence model has one ``default`` pool and stores | ||
| agents by ID. Keeping the pool check beside the lookup prevents a | ||
| future multi-pool change from turning separately validated path | ||
| parameters into an object-authorization bypass. | ||
| """ | ||
| if agent_pool_id != "default": | ||
| raise KeyError(agent_pool_id) | ||
| return self._agent(worker_agent_id) |
There was a problem hiding this comment.
📝 Info: Uniform not-found avoids cross-pool disclosure
For a wrong pool, _agent_in_pool raises KeyError(agent_pool_id) yet all verbs return the same agent_not_found 404. This deliberately hides whether the worker exists in another pool, matching the IDOR-mitigation intent.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Merge-gate evidence (2026-08-24): Deep diff review + fixes applied; all required checks green on current head except strix (org-wide NVIDIA NIM quota exhaustion — external provider-capacity blocker; serialization fix in ContextualWisdomLab/.github#1297). Full local suite green on this head. |
# Conflicts: # CHANGELOG.md
| except KeyError as exc: | ||
| self._send_error(404, "resource_not_found", str(exc)) | ||
| self._send_error(404, "agent_not_found", str(exc)) |
There was a problem hiding this comment.
📝 Info: PATCH/DELETE error message differs from GET for wrong pool
GET returns a fixed agent {worker_agent_id} not found message, while PATCH and DELETE return str(exc), which for a wrong pool is the repr of the pool id (e.g. 'other_pool'). The agent_not_found code is consistent across all three verbs, which is what the tests assert, but the message text is not.
Was this helpful? React with 👍 or 👎 to provide feedback.
| except KeyError as exc: | ||
| self._send_error(404, "resource_not_found", str(exc)) | ||
| self._send_error(404, "agent_not_found", str(exc)) |
There was a problem hiding this comment.
📝 Info: Broad KeyError-to-404 mapping in PATCH/DELETE
The except KeyError handlers map any KeyError from the whole method to 404 agent_not_found. An internal dict-access KeyError inside patch_agent/remove_agent would be masked as a not-found rather than a 500. Pre-existing behavior, unchanged in scope by this PR.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Merge-gate evidence (2026-08-24): Deep review + integration complete; all required checks green on current head except strix (org-wide NVIDIA NIM quota exhaustion — external provider-capacity blocker; serialization fix in ContextualWisdomLab/.github#1297). Full local suite green. |
…found #831 unified worker-agent not-found errors on agent_not_found; the model-group CRUD contract now asserts that same canonical code.
* feat(routing): add operator-managed model groups * fix(api): enforce model group create semantics * fix(routing): preserve eligibility and REST contracts * docs(groups): remove ephemeral model example * fix(groups): route advertised aliases end to end * ci: hourly OpenCode maintenance agent routed through this gateway (#835) * ci: add hourly OpenCode maintenance agent routed through this gateway The scheduled job boots the contextual-orchestrator gateway with the five org provider secrets seeded into the process-local KV registry (bootstrap transport only), auto-discovers chat-capable models, assigns the ox-alpha measured-routing group across OpenRouter + OpenCode Zen aliases, then runs the pinned OpenCode CLI pointed at http://127.0.0.1:8000/v1 with model 'ox-alpha' so the agent's own traffic exercises group routing. The agent works the PR queue (review -> fix -> recheck -> merge), root-causes failing checks, and advances docs/product-technical-gap-baseline.md when the PR queue is empty. COPILOT_GITHUB_TOKEN is not used; the existing review-agent key scheme is untouched. * fix(ci): remove ephemeral model binding * fix(ci): install gateway and grant branch writes * fix(groups): preserve internal dispatch default * fix: isolate model group routing evidence * fix: preserve conduct semantics for model groups * ci: minimize hourly loop permissions and installs * fix: measure streamed group routing * feat: route model groups across modalities (#837) * feat: normalize and edit model groups (#838) * fix: preserve capability routing contracts * release: v0.2.0 — model groups, cost-aware discovery, changelog baseline Bump 0.1.0 -> 0.2.0 and add the canonical Keep-a-Changelog file with the 0.1.0 baseline and the 0.2.0 additions (operator-managed model groups, measured group routing, OpenCode Zen discovery + free-tier classification, Strix B105 root-cause remediation). * fix(discovery): tolerate null modality arrays * docs: specify model group product and technical contracts * release: align v0.2.0 changelog and lock metadata * feat: stream orchestrated reasoning summaries * fix: keep free reasoning streams fail closed * fix: preserve free routing evidence and analytics * fix: distinguish failed Responses streams in analytics * fix: pin structured free judge to selected agent * fix: lock container dependencies and virtual capabilities * fix: keep free passthrough on zero-cost models * fix: prune removed routing measurements * refactor: remove unreachable responses passthrough branch * fix: route virtual models across media capabilities * fix: retain reset candidate routing rows * fix: preserve Responses instructions in workflows * test: align model-group missing-member code with canonical agent_not_found #831 unified worker-agent not-found errors on agent_not_found; the model-group CRUD contract now asserts that same canonical code. * fix(api): model-group creation returns canonical agent_not_found for unknown members * ci: route hourly OpenCode loop through auto * fix: reject unsupported orchestrated structured output * docs: correct Responses stream options error * fix: retrieve URL-encoded model identifiers * fix: measure free capability and failover routing * fix: resolve model group review findings * fix: close model group integration gaps * fix: align provider inventory and session cache scope * fix: harden compose secrets and repeated reasoning summaries * fix: contain binary response disconnects * fix: ground Zen free discovery in structured costs * fix: keep group judge within allowed members * fix: preserve free catalog evidence across reloads * docs: assign unique model-group ADR number * fix: preserve catalog capability evidence * fix: preserve declared Bytez endpoint capabilities * fix: validate batch model identity at ingress * fix(api): normalize missing model group errors * docs(prd): align product bets with model groups * fix(discovery): filter chat-only Bytez transports * docs(adr): reserve model-group decision identifier * feat: replace routing heuristics with measured evidence ledgers (#847) * feat: replace routing heuristics with measured evidence ledgers Remove DOMAIN_HINTS/COMPLEX_HINTS keyword tables; route via eligibility contracts, declaration priority/capability fit/cosine affinity over operator-declared metadata, and measured intra-group quality then EWMA tokens-per-second. Add structured fail-closed triage gas with content-hash verdict caching and real-time fast-mlsirm judging on direct routes that feeds a Beta-Bernoulli quality ledger with in-budget failover. ADR 0027 + doctoring APA 7 references (Jacobson 1988; Gelman et al. 2013; Karpukhin et al. 2020; Ong et al. 2024; Chen et al. 2023; Zheng et al. 2023; Jeon et al. 2021). Gap baseline added at docs/product-technical-gap-baseline.md. * fix: keep routing evidence units and capability boundaries honest * fix(admin): tolerate unavailable model-group state * docs(loop): require PRD and measured web capacity * fix(admin): remove retired policy hint metric * fix(routing): validate evidence before mutation * fix(discovery): remove model-name free inference guidance * fix: complete #834 model-group persistence on the normalized agent-pool schema - model_group/model_group_member relations compose with main's normalized agent_pool (no JSON shadow); save() maintains membership, load_all() restores group_name via join. - Legacy payload promotion reads agent_pool_legacy_payloads during the migration window and drops it after promoting group names. - DB-naming gate now extracts SQL from AST string constants so prose in comments can no longer produce false identifier violations. - Batch runner signature aligned (messages, mode, model) with the merged LocalBatchBackend; stream-route fake accepts the merged caller kwargs. * test: case-sensitive DDL pattern so prose cannot fake identifiers * docs: refresh protected-main gap evidence * docs: record exact provider regression suite * docs: correct exact provider head * fix(admin): make guidance customer-actionable * fix(admin): remove internal terms from customer copy * docs(ui): record customer-copy screenshot audit * test(ci): lock hourly orchestrator loop contract * fix: replace synthetic admin status with truthful empty states * docs: refresh exact-head remediation baseline * fix(stream): cancel orchestration after Responses disconnect * test: align boundary contracts with current routing * docs: complete public boundary docstrings * docs: refresh exact-head product gap queue * test: enforce complete public docstrings * ci: enforce protected exact-head merge loop
1 participant
Summary
Closes the Strix IDOR finding on worker-agent routes:
_agent_in_pool()now resolves the pool check beside object lookup, and GET/PATCH/DELETE all return the sameagent_not_foundcode for a different-pool id. No bypass path — the singledefaultpool invariant is enforced at one chokepoint.This is the narrow rebuild of #804: it contains only commits
a1f6716d+f74cfc07(the pool boundary fix), rebased onto current main. The principal-scoped evidence work and the release-authorization framework from #804 are intentionally excluded; they need independent review at that size and will be re-proposed separately (tracked for follow-up so nothing silently disappears).Tests
test_http_worker_agent_read_rejects_wrong_pool_id) asserts 404 +agent_not_foundfor GET/PATCH/DELETE.