security: gate register_agent() behind org entitlement check#655
security: gate register_agent() behind org entitlement check#655laynepenney merged 1 commit intosprint-17from
Conversation
register_agent() previously wrote to org team.db without verifying the caller had any relationship to the org. Any process knowing an org_id string could register agents. Add _check_org_entitlement() that verifies one of: - db_path explicitly passed (test/internal use) - SYNAPT_AGENT_ID env var set (spawned by gr spawn) - team.db already exists (org initialized by gr) Raises PermissionError with org_id in the message if none hold. 5 new tests covering all entitlement paths + rejection. 1981 passed, 0 failed. Closes #530 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement. To sign, please comment on this PR with the following exact text:
You can also re-trigger the CLA check by commenting I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
laynepenney
left a comment
There was a problem hiding this comment.
LGTM. Clean single-function gate at the entry point of register_agent(). The three entitlement paths are well-chosen:
db_pathbypass for tests/internal useSYNAPT_AGENT_IDenv var for gr-spawn-managed processes- Existing team.db for already-initialized orgs
The PermissionError with org_id in the message makes debugging straightforward. 5 tests cover all entitlement paths and both denial scenarios (no entitlement, error content).
This is appropriate defense-in-depth for the registration boundary. The gate belongs in the OSS registry since the function is already there; premium entitlement policy can layer stricter checks on top later.
laynepenney
left a comment
There was a problem hiding this comment.
LGTM. Clean single-function gate at the entry point of register_agent() — the right place to enforce this, before any DB writes happen.
The three entitlement tiers are well-chosen: db_path bypass for tests/internal (caller owns the path), SYNAPT_AGENT_ID for gr-spawn-managed processes (premium sets the signal, OSS honors it without importing premium), existing team.db for already-initialized orgs (least-privilege expansion). These cover the real operational scenarios without over-engineering full AAA at this stage.
Boundary declaration is correct: the guard mechanism belongs in OSS (prevent unauthorized writes at the registry layer regardless of caller), even though the identity concept being protected is premium. This is exactly the OSS/premium seam pattern — OSS exposes the hook, premium layers on top.
5 tests cover all paths and both failure modes (no entitlement, error content). PermissionError with org_id in the message makes production debugging tractable.
This PR is at 1 review — needs one more before merge.
laynepenney
left a comment
There was a problem hiding this comment.
Opus review: LGTM. Single-function entitlement gate at the top of register_agent() before any DB writes. Three entitlement tiers cover the real scenarios: explicit db_path (tests), SYNAPT_AGENT_ID (gr spawn), existing team.db (initialized org). PermissionError with org_id gives tractable debugging. OSS/premium boundary is correct: the guard mechanism is OSS, the identity concept it protects is premium.
At 2 reviews (Layne + Opus). Meets policy.
Summary
register_agent()previously wrote to orgteam.dbwithout verifying the caller had any relationship to the org_check_org_entitlement()gate that verifies one of three conditions: explicitdb_path(tests),SYNAPT_AGENT_IDenv var (gr spawn), or existingteam.db(initialized org)PermissionErrorwith org_id in the message if none holdChanges
registry.py: Added_check_org_entitlement(), called at top ofregister_agent()test_channel_scoping.py: 5 new tests covering all entitlement paths + rejection + error message contentPremium boundary: OSS recall infrastructure. Identity is premium, but the entitlement check lives at the registry boundary in OSS to prevent unauthorized writes regardless of which layer calls it.
Test plan
Closes #530
🤖 Generated with Claude Code