Repository navigation
feat(api): give every run a principal and decide access in one place - #7754
Conversation
|
🤖 The AI agent says: @coderabbitai review |
📝 Summary
Merge Risk:
|
|
Tip For best results, initiate chat on the files or code changes. @mmabrouk I couldn't produce a complete response to this request. Please try again. You are interacting with an AI system. |
Website previewPreview URL: https://pr-7754-agenta-website-preview.mahmoud-637.workers.dev Built from |
|
🤖 The AI agent says: @coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments: 1
api/oss/tests/pytest/unit/sessions/test_heartbeat_is_current_turn.py: 1
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRestore the docstring text.
A bulk edit changed the module docstring. It now reads "
heartbeat(envelope=make_envelope())'s acquire-then-refresh fallback". The original text referred to the plainheartbeat()method. The new wording reads as if the test helper owns the fallback.Proposed fix
-Before this, `heartbeat(envelope=make_envelope())`'s acquire-then-refresh fallback silently re-acquired a lost alive +Before this, `heartbeat()`'s acquire-then-refresh fallback silently re-acquired a lost alive
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Agenta-AI/agenta
- Branch: feat/m2-run-principal
- File: .coderabbit.yaml
- Commit: af7bfb2
- Layers: Repository YAML (base), Organization UI (inherited)
- Review profile: CHILL
- Plan: Advanced
- Run ID:
5ee5734c-4982-4bb5-9497-37b623318b3b
⛔ Files ignored due to path filters: 24
web/packages/agenta-api-client/src/generated/api/resources/access/client/Client.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/access/client/requests/CheckPermissionsRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/gateway/client/Client.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/gatewayMcp/client/Client.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/gatewayMcp/client/requests/McpEndpointCheckRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/sessions/client/Client.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/AgentError.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/AgentRef.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/EvaluationRun.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/McpCredentialBinding.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/PrincipalKind.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/PrincipalRef.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunApproval.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunAuthority.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunContext.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunEnvelope.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunOrigin.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunRestrictions.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunSender.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionTurn.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/TriggerDeliveryData.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/TriggerSchedule.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/TriggerSubscription.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/index.tsis excluded by!**/generated/**
📒 Files selected for processing: 197
api/AGENTS.mdapi/ee/tests/pytest/acceptance/accounts/test_project_creation_seeding_ee.pyapi/entrypoints/access.pyapi/entrypoints/channel_adapters.pyapi/entrypoints/routers.pyapi/entrypoints/worker_queues.pyapi/oss/databases/postgres/migrations/core_oss/versions/oss000000046_add_run_principal_columns.pyapi/oss/databases/postgres/migrations/core_oss/versions/oss000000047_backfill_run_principal.pyapi/oss/src/apis/fastapi/access/router.pyapi/oss/src/apis/fastapi/agent_templates/router.pyapi/oss/src/apis/fastapi/evaluations/router.pyapi/oss/src/apis/fastapi/gateways/credentials_router.pyapi/oss/src/apis/fastapi/gateways/mcps/router.pyapi/oss/src/apis/fastapi/sessions/router.pyapi/oss/src/apis/fastapi/tools/router.pyapi/oss/src/apis/fastapi/triggers/router.pyapi/oss/src/core/access/context.pyapi/oss/src/core/access/dtos.pyapi/oss/src/core/access/interfaces.pyapi/oss/src/core/access/permissions/service.pyapi/oss/src/core/access/principals.pyapi/oss/src/core/access/service.pyapi/oss/src/core/access/types.pyapi/oss/src/core/accounts/service.pyapi/oss/src/core/agent_templates/loader.pyapi/oss/src/core/channels/queue.pyapi/oss/src/core/channels/service.pyapi/oss/src/core/evaluations/interfaces.pyapi/oss/src/core/evaluations/run_access.pyapi/oss/src/core/evaluations/runtime/adapters.pyapi/oss/src/core/evaluations/service.pyapi/oss/src/core/evaluations/tasks/processor.pyapi/oss/src/core/evaluations/tasks/run.pyapi/oss/src/core/evaluations/types.pyapi/oss/src/core/memberships/dtos.pyapi/oss/src/core/memberships/service.pyapi/oss/src/core/sessions/commands/delivery.pyapi/oss/src/core/sessions/commands/dtos.pyapi/oss/src/core/sessions/commands/interfaces.pyapi/oss/src/core/sessions/commands/service.pyapi/oss/src/core/sessions/inputs/dtos.pyapi/oss/src/core/sessions/inputs/service.pyapi/oss/src/core/sessions/interactions/dtos.pyapi/oss/src/core/sessions/interactions/service.pyapi/oss/src/core/sessions/starts/service.pyapi/oss/src/core/sessions/streams/service.pyapi/oss/src/core/sessions/streams/types.pyapi/oss/src/core/sessions/turns/dtos.pyapi/oss/src/core/tools/platform_handlers.pyapi/oss/src/core/triggers/dtos.pyapi/oss/src/core/triggers/interfaces.pyapi/oss/src/core/triggers/service.pyapi/oss/src/core/workflows/dtos.pyapi/oss/src/core/workflows/service.pyapi/oss/src/dbs/http/sessions/control_delivery_direct.pyapi/oss/src/dbs/postgres/access/__init__.pyapi/oss/src/dbs/postgres/access/dao.pyapi/oss/src/dbs/postgres/access/mappings.pyapi/oss/src/dbs/postgres/evaluations/dao.pyapi/oss/src/dbs/postgres/evaluations/dbas.pyapi/oss/src/dbs/postgres/evaluations/mappings.pyapi/oss/src/dbs/postgres/sessions/commands/dbas.pyapi/oss/src/dbs/postgres/sessions/commands/mappings.pyapi/oss/src/dbs/postgres/sessions/inputs/dao.pyapi/oss/src/dbs/postgres/sessions/inputs/dbes.pyapi/oss/src/dbs/postgres/sessions/inputs/mappings.pyapi/oss/src/dbs/postgres/sessions/interactions/dbas.pyapi/oss/src/dbs/postgres/sessions/interactions/mappings.pyapi/oss/src/dbs/postgres/sessions/turns/dao.pyapi/oss/src/dbs/postgres/sessions/turns/dbas.pyapi/oss/src/dbs/postgres/sessions/turns/mappings.pyapi/oss/src/dbs/postgres/triggers/dao.pyapi/oss/src/dbs/postgres/triggers/dbas.pyapi/oss/src/dbs/postgres/triggers/mappings.pyapi/oss/src/middlewares/auth.pyapi/oss/src/routers/projects_router.pyapi/oss/src/services/api_key_service.pyapi/oss/src/services/user_service.pyapi/oss/src/tasks/asyncio/channels/inbox.pyapi/oss/src/tasks/asyncio/sessions/interactions_dispatcher.pyapi/oss/src/tasks/asyncio/triggers/dispatcher.pyapi/oss/src/tasks/taskiq/sessions/interactions_worker.pyapi/oss/tests/pytest/acceptance/accounts/test_project_route_permissions.pyapi/oss/tests/pytest/acceptance/sessions/test_run_principal_acceptance.pyapi/oss/tests/pytest/integration/access/__init__.pyapi/oss/tests/pytest/integration/access/conftest.pyapi/oss/tests/pytest/integration/access/test_principals_dao_postgres.pyapi/oss/tests/pytest/integration/access/test_run_principal_query_counts_postgres.pyapi/oss/tests/pytest/integration/channels/conftest.pyapi/oss/tests/pytest/integration/gateways/test_mcp_oauth_connect.pyapi/oss/tests/pytest/integration/memberships/test_admin_membership_invalidation_postgres.pyapi/oss/tests/pytest/integration/memberships/test_hot_path_query_counts_postgres.pyapi/oss/tests/pytest/integration/memberships/test_membership_parity_postgres.pyapi/oss/tests/pytest/integration/migrations/__init__.pyapi/oss/tests/pytest/integration/migrations/conftest.pyapi/oss/tests/pytest/integration/migrations/test_run_principal_backfill_postgres.pyapi/oss/tests/pytest/integration/migrations/test_run_principal_columns_rerun_postgres.pyapi/oss/tests/pytest/integration/sessions/test_refused_continuations_postgres.pyapi/oss/tests/pytest/integration/sessions/test_run_envelope_columns_postgres.pyapi/oss/tests/pytest/integration/triggers/__init__.pyapi/oss/tests/pytest/integration/triggers/conftest.pyapi/oss/tests/pytest/integration/triggers/test_triggers_refuse_postgres.pyapi/oss/tests/pytest/unit/access/test_access_boundaries.pyapi/oss/tests/pytest/unit/access/test_access_service.pyapi/oss/tests/pytest/unit/access/test_acting_user.pyapi/oss/tests/pytest/unit/access/test_check_action_access_wrapper.pyapi/oss/tests/pytest/unit/access/test_decide.pyapi/oss/tests/pytest/unit/access/test_grant_exchange.pyapi/oss/tests/pytest/unit/access/test_principal_invalidation.pyapi/oss/tests/pytest/unit/access/test_principals_cache.pyapi/oss/tests/pytest/unit/access/test_run_principal_dtos.pyapi/oss/tests/pytest/unit/access/test_run_principal_mappings.pyapi/oss/tests/pytest/unit/agent_templates/conftest.pyapi/oss/tests/pytest/unit/agent_templates/test_github_loading.pyapi/oss/tests/pytest/unit/agent_templates/test_loader.pyapi/oss/tests/pytest/unit/agent_templates/test_router.pyapi/oss/tests/pytest/unit/channels/conftest.pyapi/oss/tests/pytest/unit/channels/test_channels_approval_admission.pyapi/oss/tests/pytest/unit/channels/test_channels_inbox_dispatcher.pyapi/oss/tests/pytest/unit/channels/test_channels_inbox_principal.pyapi/oss/tests/pytest/unit/channels/test_channels_live_qa_regressions.pyapi/oss/tests/pytest/unit/channels/test_channels_rechecks.pyapi/oss/tests/pytest/unit/channels/test_channels_turn_context.pyapi/oss/tests/pytest/unit/evaluations/conftest.pyapi/oss/tests/pytest/unit/evaluations/test_query_eval_loops.pyapi/oss/tests/pytest/unit/evaluations/test_run_batch_invocation.pyapi/oss/tests/pytest/unit/evaluations/test_run_principal.pyapi/oss/tests/pytest/unit/evaluations/test_run_slice_ops.pyapi/oss/tests/pytest/unit/evaluations/test_runtime_topology_planner.pyapi/oss/tests/pytest/unit/gateways/conftest.pyapi/oss/tests/pytest/unit/gateways/test_gateway_credentials_envelope.pyapi/oss/tests/pytest/unit/gateways/test_gateways_mcp_oauth_callback_authorization.pyapi/oss/tests/pytest/unit/gateways/test_gateways_mcp_router.pyapi/oss/tests/pytest/unit/gateways/test_gateways_plane_flags.pyapi/oss/tests/pytest/unit/gateways/test_gateways_rollout_switches.pyapi/oss/tests/pytest/unit/gateways/test_gateways_sandbox_credential.pyapi/oss/tests/pytest/unit/memberships/test_membership_cache.pyapi/oss/tests/pytest/unit/memberships/test_memberships_invalidation.pyapi/oss/tests/pytest/unit/middlewares/test_auth_grants.pyapi/oss/tests/pytest/unit/middlewares/test_auth_run_principal.pyapi/oss/tests/pytest/unit/middlewares/test_auth_secret_token.pyapi/oss/tests/pytest/unit/migrations/test_migration_gate.pyapi/oss/tests/pytest/unit/sessions/conftest.pyapi/oss/tests/pytest/unit/sessions/test_cancel_stop_guard.pyapi/oss/tests/pytest/unit/sessions/test_command_claim_unknown_kind.pyapi/oss/tests/pytest/unit/sessions/test_command_settle_unknown_kind.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_first_touch_race.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_is_current_turn.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_lock_races.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_mirrors_existing_row.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_parked_zombie.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_refused_turn_final_beat.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_replica_id_control_chars.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_run_credential.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_runner_token.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_stale_turn_end.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_turn_binding.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_turn_handover.pyapi/oss/tests/pytest/unit/sessions/test_inputs_run_envelope.pyapi/oss/tests/pytest/unit/sessions/test_interaction_continuation_admission.pyapi/oss/tests/pytest/unit/sessions/test_interactions_dispatcher.pyapi/oss/tests/pytest/unit/sessions/test_live_relay.pyapi/oss/tests/pytest/unit/sessions/test_pending_inputs_service.pyapi/oss/tests/pytest/unit/sessions/test_respond_interaction_durable.pyapi/oss/tests/pytest/unit/sessions/test_session_commands_dao.pyapi/oss/tests/pytest/unit/sessions/test_session_inputs_dao.pyapi/oss/tests/pytest/unit/sessions/test_session_start_service.pyapi/oss/tests/pytest/unit/sessions/test_stream_fill_once.pyapi/oss/tests/pytest/unit/sessions/test_stream_header_merge.pyapi/oss/tests/pytest/unit/sessions/test_turn_attribution.pyapi/oss/tests/pytest/unit/sessions/test_watch_interactions_publish.pyapi/oss/tests/pytest/unit/sessions/test_watch_lifecycle_publish.pyapi/oss/tests/pytest/unit/sessions/test_watch_session_list_publish.pyapi/oss/tests/pytest/unit/test_channel_adapters_entrypoint.pyapi/oss/tests/pytest/unit/tools/conftest.pyapi/oss/tests/pytest/unit/tools/test_platform_handlers.pyapi/oss/tests/pytest/unit/tools/test_workflow_tool_call.pyapi/oss/tests/pytest/unit/triggers/conftest.pyapi/oss/tests/pytest/unit/triggers/test_triggers_dispatcher.pyapi/oss/tests/pytest/unit/triggers/test_triggers_lifecycle.pyapi/oss/tests/pytest/unit/triggers/test_triggers_principal.pyapi/oss/tests/pytest/unit/triggers/test_triggers_schedules_refresh.pyapi/oss/tests/pytest/unit/triggers/test_triggers_soft_delete_postgres.pyapi/oss/tests/pytest/unit/triggers/test_triggers_subscription_test_mode.pyapi/oss/tests/pytest/unit/vault/test_write_only_routes.pyapi/oss/tests/pytest/unit/workflows/conftest.pyapi/oss/tests/pytest/unit/workflows/test_archive_pauses_triggers.pyapi/oss/tests/pytest/unit/workflows/test_invoke_detached.pyapi/oss/tests/pytest/unit/workflows/test_prepare_invoke_envelope.pyapi/oss/tests/pytest/unit/workflows/test_service_url_policy.pyapi/oss/tests/pytest/utils/cache.pyapi/oss/tests/pytest/utils/memberships.pyapi/oss/tests/pytest/utils/principals.pyapi/oss/tests/pytest/utils/statements.pysdks/python/agenta/sdk/middlewares/routing/auth.pysdks/python/oss/tests/pytest/unit/test_auth_middleware_credentials.pyservices/oss/tests/pytest/unit/agent/test_credential_exchange.py
💤 Files with no reviewable changes: 1
- services/oss/tests/pytest/unit/agent/test_credential_exchange.py
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
|
🤖 The AI agent says: Round 1: all 7 findings and the nitpick (docstring restored) are fixed in 39ab8ee; threads answered and resolved. @coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Agenta-AI/agenta
- Branch: feat/m2-run-principal
- File: .coderabbit.yaml
- Commit: 39ab8ee
- Layers: Repository YAML (base), Organization UI (inherited)
- Review profile: CHILL
- Plan: Advanced
- Run ID:
abaa724f-e25a-43b4-ae36-e491280a37cd
📒 Files selected for processing: 10
api/oss/src/core/access/service.pyapi/oss/src/core/evaluations/interfaces.pyapi/oss/src/core/sessions/streams/service.pyapi/oss/src/core/workflows/service.pyapi/oss/src/tasks/asyncio/channels/inbox.pyapi/oss/src/tasks/asyncio/triggers/dispatcher.pyapi/oss/tests/pytest/unit/channels/test_channels_rechecks.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_is_current_turn.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_run_credential.pyapi/oss/tests/pytest/unit/workflows/test_archive_pauses_triggers.py
🚧 Files skipped from review as they are similar to previous changes: 2
- api/oss/tests/pytest/unit/sessions/test_heartbeat_is_current_turn.py
- api/oss/src/tasks/asyncio/triggers/dispatcher.py
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
|
🤖 The AI agent says: Latency, network round trips and database load after the gate fixes (head What changed since the latency gate failedThe gate measured session send
API calls per session send (deterministic)These are API access-log lines in the window of 10 sends. Each side is a separate services process of the same stack (same API, same database). "before" runs the services SDK of
"before" makes one exchange per send. "after" makes 0 per send on a warm cache, as M1 does. The few calls left on "after" are the cold first call and other traffic on the shared stack (the EE Bearer window had extra background traffic: 324 API lines against about 230 in the other windows). The rest of a send's API calls (about 22) are the same on both sides. Session send latency (time to response headers)The runs are interleaved before/after, 3 rounds, N=20 after 5 warm-up sends, ApiKey and Bearer, OSS and EE. Values are the median over the rounds, in ms.
The box load stayed high during the whole run: the 1-minute load average was 2.3 to 58.6, with a median of 15, from other agents' test suites. The rounds vary by up to 60 ms between each other, so these latency numbers cannot resolve a 10 ms effect. The call counts above are the measurement that answers the gate: the extra round trip is gone. Network round trips added by M2 per request (compared with M1)
M2 adds no HTTP call (checked over the full diff). Two Redis reads remain per turn, not per request: the gateway-credential eligibility check and, in worker-started runs, the acting-user read. Both are cached reads with no Postgres statement. Database load (statements per request and per turn)Measured by the DB-load check on pinned base
All warm paths are equal in base, M1 and M2. The extra cold reads in the M2 column ( Two items are recorded and not changed:
Evidence (local paths): |
|
🤖 The AI agent says: New commits since round 2 (services cache, 403 mapping, one MGET per decision, request memo, folded evaluation UPDATE). @coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Agenta-AI/agenta
- Branch: feat/m2-run-principal
- File: .coderabbit.yaml
- Commit: b3fe813
- Layers: Repository YAML (base), Organization UI (inherited)
- Review profile: CHILL
- Plan: Advanced
- Run ID:
674835d5-f922-48ef-a07b-6d73c7964137
📒 Files selected for processing: 21
api/ee/src/core/access/rbac.pyapi/oss/src/core/access/context.pyapi/oss/src/core/access/interfaces.pyapi/oss/src/core/access/service.pyapi/oss/src/core/evaluations/service.pyapi/oss/src/core/memberships/service.pyapi/oss/src/dbs/postgres/access/dao.pyapi/oss/src/dbs/postgres/evaluations/dao.pyapi/oss/src/middlewares/auth.pyapi/oss/src/utils/caching.pyapi/oss/tests/pytest/integration/access/test_run_principal_query_counts_postgres.pyapi/oss/tests/pytest/unit/evaluations/test_run_principal.pyapi/oss/tests/pytest/unit/middlewares/test_auth_run_principal.pyapi/oss/tests/pytest/utils/cache.pysdks/python/agenta/sdk/agents/connections/__init__.pysdks/python/agenta/sdk/agents/connections/errors.pysdks/python/agenta/sdk/agents/platform/connections.pysdks/python/agenta/sdk/middlewares/routing/auth.pysdks/python/agenta/sdk/middlewares/running/normalizer.pysdks/python/oss/tests/pytest/unit/agents/platform/test_connections_http.pysdks/python/oss/tests/pytest/unit/test_auth_middleware_credentials.py
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review.
📘 Docs preview
Pages changed in this pull request
This comment updates in place on every push. |
cac4d3a to
2f42ffa
Compare
743bc49 to
63a8b4d
Compare
|
🤖 The AI agent says: Rebased onto In |
Railway Preview Environment
PR closed or converted to draft. Environment absence verified; retained storage requires separate provider accounting. Post |
2f42ffa to
ddfbaf6
Compare
adc6980 to
0319338
Compare
|
🤖 The AI agent says: This PR is rebased again onto the new tip, on top of #7748. There were no conflicts. Two commits are added:
Each commit includes a unit test. The API unit layers pass for OSS and EE. |
0319338 to
3f0e23c
Compare
…luation access before step validation
…installation's linked user on evaluations
…rover on the turn, refuse removed members before a run starts
…run credential with 403 access_denied
… once per request
…es cache TTL, no repeated key read, principal in the start UPDATE)
…r, 0.124.0 upgrade notes
The events stream rechecks VIEW_SESSIONS every live_auth_recheck_seconds. Its generator runs in a copy of the request context and shared the access memo that the check at connect filled, so a recheck answered from that memo. Each check now runs with a fresh memo (fresh_request_memo) and reads the current principal and membership.
The release already resolves the test_run delta with PARAMETERS_ONLY and answers an out-of-scope change with PlatformToolHandlerRefused, so this commit keeps that code and adds tests only: an ordered set or merge on url, uri or headers is refused before any invoke, and an ordered change inside parameters still runs. The fake resolver applies the real change-set engine with the scope it receives. The release's two test_run tests pass the run envelope that test_run needs since M2.
…run engine result
On EE the owner's organization is now pinned to a plan with the RBAC entitlement and no project limit, so the role-change test checks the role rule and the two tests that create a second project can create it.
83995d7 to
6f5366b
Compare
3f0e23c to
2e143ac
Compare
|
🤖 The AI agent says: This PR is rebased onto a76ec79, on top of #7748. Head: 2e143ac. Since #7782, the release resolves the
|
Invite, resend, role change, member removal and workspace update already check the workspace's default project, so an API key of another project is refused. Organization rename, ownership transfer, deletion and workspace creation, the EE domain and SSO provider writes, and the EE billing writes now apply the same rule through check_organization_write, which runs check_action_access on the organization's default project for an API key or a run credential. Browser sessions are not confined. get_default_project_by_organization_id now returns the oldest default project, so the answer is stable for an organization with several workspaces.
…ization's default project These routes check the workspace's default project and then change the member across the whole organization. An API key or a run credential must now also be of the organization's default project (check_organization_write). Browser sessions are not affected.
2e143ac to
40ed26d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments: 1
api/oss/src/core/triggers/interfaces.py: 1
235-267:Trivial · Maintainability & Code Quality · Quick win Mark
refuse_delivery,pause_trigger, andpause_triggers_for_artifactas@abstractmethod.These three methods raise
NotImplementedErrorbut are not abstract. Every other method onTriggersDAOInterfaceis abstract. The previous review madeEvaluationsDAOInterface.set_run_principalabstract for the same reason.A DAO that omits one of these methods instantiates without error and fails later:
WorkflowsService.archive_workflowwrapspause_triggers_for_artifactinexcept Exception. A missing implementation is logged as a warning, and triggers stay active. The dispatcher still refuses them later, but only per tick.- The dispatcher fails at runtime when it calls
refuse_delivery.An abstract declaration makes an incomplete DAO fail at instantiation.
Proposed change
+ @abstractmethod async def refuse_delivery( @@ + @abstractmethod async def pause_trigger( @@ + @abstractmethod async def pause_triggers_for_artifact(Test fakes that subclass the interface must implement the three methods.

Review info
-
Run configuration-
- Source mode: Auto
- Repository: Agenta-AI/agenta
- Branch: feat/m2-run-principal
- File: .coderabbit.yaml
- Commit: 40ed26d
- Layers: Repository YAML (base), Organization UI (inherited)
- Review profile: CHILL
- Plan: Advanced
- Run ID:
c6360a09-5533-4b35-b1be-bd61961a036e
Files ignored due to path filters: 24-
web/packages/agenta-api-client/src/generated/api/resources/access/client/Client.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/access/client/requests/CheckPermissionsRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/gateway/client/Client.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/gatewayMcp/client/Client.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/gatewayMcp/client/requests/McpEndpointCheckRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/sessions/client/Client.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/AgentError.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/AgentRef.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/EvaluationRun.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/McpCredentialBinding.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/PrincipalKind.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/PrincipalRef.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunApproval.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunAuthority.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunContext.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunEnvelope.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunOrigin.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunRestrictions.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/RunSender.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionTurn.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/TriggerDeliveryData.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/TriggerSchedule.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/TriggerSubscription.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/index.tsis excluded by!**/generated/**
Files selected for processing: 219-
api/AGENTS.mdapi/ee/src/apis/fastapi/billing/router.pyapi/ee/src/apis/fastapi/organizations/router.pyapi/ee/src/core/access/rbac.pyapi/ee/tests/pytest/acceptance/accounts/test_project_creation_seeding_ee.pyapi/entrypoints/access.pyapi/entrypoints/channel_adapters.pyapi/entrypoints/routers.pyapi/entrypoints/worker_queues.pyapi/oss/databases/postgres/migrations/core_oss/versions/oss000000046_add_run_principal_columns.pyapi/oss/databases/postgres/migrations/core_oss/versions/oss000000047_backfill_run_principal.pyapi/oss/src/apis/fastapi/access/router.pyapi/oss/src/apis/fastapi/agent_templates/router.pyapi/oss/src/apis/fastapi/evaluations/router.pyapi/oss/src/apis/fastapi/gateways/credentials_router.pyapi/oss/src/apis/fastapi/gateways/mcps/router.pyapi/oss/src/apis/fastapi/sessions/router.pyapi/oss/src/apis/fastapi/tools/router.pyapi/oss/src/apis/fastapi/triggers/router.pyapi/oss/src/core/access/context.pyapi/oss/src/core/access/dtos.pyapi/oss/src/core/access/interfaces.pyapi/oss/src/core/access/permissions/service.pyapi/oss/src/core/access/principals.pyapi/oss/src/core/access/service.pyapi/oss/src/core/access/types.pyapi/oss/src/core/accounts/service.pyapi/oss/src/core/agent_templates/loader.pyapi/oss/src/core/channels/queue.pyapi/oss/src/core/channels/service.pyapi/oss/src/core/evaluations/interfaces.pyapi/oss/src/core/evaluations/run_access.pyapi/oss/src/core/evaluations/runtime/adapters.pyapi/oss/src/core/evaluations/service.pyapi/oss/src/core/evaluations/tasks/processor.pyapi/oss/src/core/evaluations/tasks/run.pyapi/oss/src/core/evaluations/types.pyapi/oss/src/core/memberships/dtos.pyapi/oss/src/core/memberships/service.pyapi/oss/src/core/sessions/commands/delivery.pyapi/oss/src/core/sessions/commands/dtos.pyapi/oss/src/core/sessions/commands/interfaces.pyapi/oss/src/core/sessions/commands/service.pyapi/oss/src/core/sessions/inputs/dtos.pyapi/oss/src/core/sessions/inputs/service.pyapi/oss/src/core/sessions/interactions/dtos.pyapi/oss/src/core/sessions/interactions/service.pyapi/oss/src/core/sessions/starts/service.pyapi/oss/src/core/sessions/streams/service.pyapi/oss/src/core/sessions/streams/types.pyapi/oss/src/core/sessions/turns/dtos.pyapi/oss/src/core/tools/platform_handlers.pyapi/oss/src/core/triggers/dtos.pyapi/oss/src/core/triggers/interfaces.pyapi/oss/src/core/triggers/service.pyapi/oss/src/core/workflows/dtos.pyapi/oss/src/core/workflows/service.pyapi/oss/src/dbs/http/sessions/control_delivery_direct.pyapi/oss/src/dbs/postgres/access/__init__.pyapi/oss/src/dbs/postgres/access/dao.pyapi/oss/src/dbs/postgres/access/mappings.pyapi/oss/src/dbs/postgres/evaluations/dao.pyapi/oss/src/dbs/postgres/evaluations/dbas.pyapi/oss/src/dbs/postgres/evaluations/mappings.pyapi/oss/src/dbs/postgres/sessions/commands/dbas.pyapi/oss/src/dbs/postgres/sessions/commands/mappings.pyapi/oss/src/dbs/postgres/sessions/inputs/dao.pyapi/oss/src/dbs/postgres/sessions/inputs/dbes.pyapi/oss/src/dbs/postgres/sessions/inputs/mappings.pyapi/oss/src/dbs/postgres/sessions/interactions/dbas.pyapi/oss/src/dbs/postgres/sessions/interactions/mappings.pyapi/oss/src/dbs/postgres/sessions/turns/dao.pyapi/oss/src/dbs/postgres/sessions/turns/dbas.pyapi/oss/src/dbs/postgres/sessions/turns/mappings.pyapi/oss/src/dbs/postgres/triggers/dao.pyapi/oss/src/dbs/postgres/triggers/dbas.pyapi/oss/src/dbs/postgres/triggers/mappings.pyapi/oss/src/middlewares/auth.pyapi/oss/src/routers/organization_router.pyapi/oss/src/routers/projects_router.pyapi/oss/src/routers/workspace_router.pyapi/oss/src/services/api_key_service.pyapi/oss/src/services/db_manager.pyapi/oss/src/services/user_service.pyapi/oss/src/tasks/asyncio/channels/inbox.pyapi/oss/src/tasks/asyncio/sessions/interactions_dispatcher.pyapi/oss/src/tasks/asyncio/triggers/dispatcher.pyapi/oss/src/tasks/taskiq/sessions/interactions_worker.pyapi/oss/src/utils/caching.pyapi/oss/tests/pytest/acceptance/accounts/test_organizations.pyapi/oss/tests/pytest/acceptance/accounts/test_project_route_permissions.pyapi/oss/tests/pytest/acceptance/sessions/test_run_principal_acceptance.pyapi/oss/tests/pytest/integration/access/__init__.pyapi/oss/tests/pytest/integration/access/conftest.pyapi/oss/tests/pytest/integration/access/test_principals_dao_postgres.pyapi/oss/tests/pytest/integration/access/test_run_principal_query_counts_postgres.pyapi/oss/tests/pytest/integration/channels/conftest.pyapi/oss/tests/pytest/integration/gateways/test_mcp_oauth_connect.pyapi/oss/tests/pytest/integration/memberships/test_admin_membership_invalidation_postgres.pyapi/oss/tests/pytest/integration/memberships/test_hot_path_query_counts_postgres.pyapi/oss/tests/pytest/integration/memberships/test_membership_parity_postgres.pyapi/oss/tests/pytest/integration/migrations/__init__.pyapi/oss/tests/pytest/integration/migrations/conftest.pyapi/oss/tests/pytest/integration/migrations/test_run_principal_backfill_postgres.pyapi/oss/tests/pytest/integration/migrations/test_run_principal_columns_rerun_postgres.pyapi/oss/tests/pytest/integration/sessions/test_refused_continuations_postgres.pyapi/oss/tests/pytest/integration/sessions/test_run_envelope_columns_postgres.pyapi/oss/tests/pytest/integration/triggers/__init__.pyapi/oss/tests/pytest/integration/triggers/conftest.pyapi/oss/tests/pytest/integration/triggers/test_triggers_refuse_postgres.pyapi/oss/tests/pytest/unit/access/test_access_boundaries.pyapi/oss/tests/pytest/unit/access/test_access_service.pyapi/oss/tests/pytest/unit/access/test_acting_user.pyapi/oss/tests/pytest/unit/access/test_check_action_access_wrapper.pyapi/oss/tests/pytest/unit/access/test_decide.pyapi/oss/tests/pytest/unit/access/test_grant_exchange.pyapi/oss/tests/pytest/unit/access/test_organization_write.pyapi/oss/tests/pytest/unit/access/test_principal_invalidation.pyapi/oss/tests/pytest/unit/access/test_principals_cache.pyapi/oss/tests/pytest/unit/access/test_run_principal_dtos.pyapi/oss/tests/pytest/unit/access/test_run_principal_mappings.pyapi/oss/tests/pytest/unit/agent_templates/conftest.pyapi/oss/tests/pytest/unit/agent_templates/test_github_loading.pyapi/oss/tests/pytest/unit/agent_templates/test_loader.pyapi/oss/tests/pytest/unit/agent_templates/test_router.pyapi/oss/tests/pytest/unit/channels/conftest.pyapi/oss/tests/pytest/unit/channels/test_channels_approval_admission.pyapi/oss/tests/pytest/unit/channels/test_channels_inbox_dispatcher.pyapi/oss/tests/pytest/unit/channels/test_channels_inbox_principal.pyapi/oss/tests/pytest/unit/channels/test_channels_live_qa_regressions.pyapi/oss/tests/pytest/unit/channels/test_channels_rechecks.pyapi/oss/tests/pytest/unit/channels/test_channels_turn_context.pyapi/oss/tests/pytest/unit/evaluations/conftest.pyapi/oss/tests/pytest/unit/evaluations/test_query_eval_loops.pyapi/oss/tests/pytest/unit/evaluations/test_run_batch_invocation.pyapi/oss/tests/pytest/unit/evaluations/test_run_principal.pyapi/oss/tests/pytest/unit/evaluations/test_run_slice_ops.pyapi/oss/tests/pytest/unit/evaluations/test_runtime_topology_planner.pyapi/oss/tests/pytest/unit/gateways/conftest.pyapi/oss/tests/pytest/unit/gateways/test_gateway_credentials_envelope.pyapi/oss/tests/pytest/unit/gateways/test_gateways_mcp_oauth_callback_authorization.pyapi/oss/tests/pytest/unit/gateways/test_gateways_mcp_router.pyapi/oss/tests/pytest/unit/gateways/test_gateways_plane_flags.pyapi/oss/tests/pytest/unit/gateways/test_gateways_rollout_switches.pyapi/oss/tests/pytest/unit/gateways/test_gateways_sandbox_credential.pyapi/oss/tests/pytest/unit/memberships/test_membership_cache.pyapi/oss/tests/pytest/unit/memberships/test_memberships_invalidation.pyapi/oss/tests/pytest/unit/middlewares/test_auth_grants.pyapi/oss/tests/pytest/unit/middlewares/test_auth_run_principal.pyapi/oss/tests/pytest/unit/middlewares/test_auth_secret_token.pyapi/oss/tests/pytest/unit/migrations/test_migration_gate.pyapi/oss/tests/pytest/unit/routers/test_membership_endpoints.pyapi/oss/tests/pytest/unit/sessions/conftest.pyapi/oss/tests/pytest/unit/sessions/test_cancel_stop_guard.pyapi/oss/tests/pytest/unit/sessions/test_command_claim_unknown_kind.pyapi/oss/tests/pytest/unit/sessions/test_command_settle_unknown_kind.pyapi/oss/tests/pytest/unit/sessions/test_events_recheck_fresh_access.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_first_touch_race.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_is_current_turn.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_lock_races.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_mirrors_existing_row.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_parked_zombie.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_refused_turn_final_beat.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_replica_id_control_chars.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_run_credential.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_runner_token.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_stale_turn_end.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_turn_binding.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_turn_handover.pyapi/oss/tests/pytest/unit/sessions/test_inputs_run_envelope.pyapi/oss/tests/pytest/unit/sessions/test_interaction_continuation_admission.pyapi/oss/tests/pytest/unit/sessions/test_interactions_dispatcher.pyapi/oss/tests/pytest/unit/sessions/test_live_relay.pyapi/oss/tests/pytest/unit/sessions/test_pending_inputs_service.pyapi/oss/tests/pytest/unit/sessions/test_respond_interaction_durable.pyapi/oss/tests/pytest/unit/sessions/test_session_commands_dao.pyapi/oss/tests/pytest/unit/sessions/test_session_inputs_dao.pyapi/oss/tests/pytest/unit/sessions/test_session_start_service.pyapi/oss/tests/pytest/unit/sessions/test_stream_fill_once.pyapi/oss/tests/pytest/unit/sessions/test_stream_header_merge.pyapi/oss/tests/pytest/unit/sessions/test_turn_attribution.pyapi/oss/tests/pytest/unit/sessions/test_watch_interactions_publish.pyapi/oss/tests/pytest/unit/sessions/test_watch_lifecycle_publish.pyapi/oss/tests/pytest/unit/sessions/test_watch_session_list_publish.pyapi/oss/tests/pytest/unit/test_channel_adapters_entrypoint.pyapi/oss/tests/pytest/unit/tools/conftest.pyapi/oss/tests/pytest/unit/tools/test_platform_handlers.pyapi/oss/tests/pytest/unit/tools/test_workflow_tool_call.pyapi/oss/tests/pytest/unit/triggers/conftest.pyapi/oss/tests/pytest/unit/triggers/test_triggers_dispatcher.pyapi/oss/tests/pytest/unit/triggers/test_triggers_lifecycle.pyapi/oss/tests/pytest/unit/triggers/test_triggers_principal.pyapi/oss/tests/pytest/unit/triggers/test_triggers_schedules_refresh.pyapi/oss/tests/pytest/unit/triggers/test_triggers_soft_delete_postgres.pyapi/oss/tests/pytest/unit/triggers/test_triggers_subscription_test_mode.pyapi/oss/tests/pytest/unit/vault/test_write_only_routes.pyapi/oss/tests/pytest/unit/workflows/conftest.pyapi/oss/tests/pytest/unit/workflows/test_archive_pauses_triggers.pyapi/oss/tests/pytest/unit/workflows/test_invoke_detached.pyapi/oss/tests/pytest/unit/workflows/test_prepare_invoke_envelope.pyapi/oss/tests/pytest/unit/workflows/test_service_url_policy.pyapi/oss/tests/pytest/utils/cache.pyapi/oss/tests/pytest/utils/memberships.pyapi/oss/tests/pytest/utils/principals.pyapi/oss/tests/pytest/utils/statements.pydocs/docs/administration/access-control/01-organizations.mdxdocs/docs/administration/access-control/03-rbac.mdxdocs/docs/administration/security/01-overview.mdxdocs/docs/concepts/07-automations.mdxdocs/docs/misc/07-errors.mdxdocs/docs/reference/api-guide/01-overview.mdxdocs/docs/self-host/upgrade-and-migrate/01-upgrading.mdxsdks/python/agenta/sdk/agents/connections/__init__.pysdks/python/agenta/sdk/agents/connections/errors.pysdks/python/agenta/sdk/agents/platform/connections.pysdks/python/agenta/sdk/middlewares/routing/auth.pysdks/python/agenta/sdk/middlewares/running/normalizer.pysdks/python/oss/tests/pytest/unit/agents/platform/test_connections_http.pysdks/python/oss/tests/pytest/unit/test_auth_middleware_credentials.py
Files with no reviewable changes: 3-
- api/oss/tests/pytest/integration/migrations/init.py
- api/oss/tests/pytest/integration/triggers/init.py
- api/oss/tests/pytest/integration/access/init.py
Files skipped from review as they are similar to previous changes: 1-
- api/oss/src/dbs/postgres/access/init.py
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
-
| # The RBAC entitlement of an organization changes with its plan, which is rare. A | ||
| # decision reads it on every call; this process-local memo keeps that read off the hot | ||
| # path (no Redis round trip per decision). It is at most 30 s old, which is fresher | ||
| # than the 300 s cached decision M1 kept for the same answer. | ||
| _ENFORCED_TTL_SECONDS = 30.0 | ||
| _MEMO_CAPACITY = 4096 | ||
|
|
||
|
|
||
| class EntitlementsRbacEnforcement(RbacEnforcementInterface): | ||
| """EE: role checks apply only when the organization's plan has the RBAC | ||
| entitlement. Without it every member passes, as before.""" | ||
|
|
||
| def __init__(self) -> None: | ||
| self._memo: Dict[UUID, Tuple[float, bool]] = {} | ||
|
|
||
| async def is_enforced(self, *, organization_id: UUID) -> bool: | ||
| now = monotonic() | ||
| cached = self._memo.get(organization_id) | ||
| if cached is not None and now - cached[0] < _ENFORCED_TTL_SECONDS: | ||
| return cached[1] | ||
| check, _, _ = await check_entitlements( | ||
| key=Flag.RBAC, | ||
| scope=scope_from(organization_id=organization_id), | ||
| ) | ||
| if len(self._memo) >= _MEMO_CAPACITY: | ||
| self._memo.clear() | ||
| self._memo[organization_id] = (now, bool(check)) |
There was a problem hiding this comment.
Document or invalidate the process-local RBAC entitlement memo.
api/AGENTS.md limits caching to three items: the principal record, the membership, and the project's organization. Their writers invalidate them. This memo adds a fourth cached input to decide, and nothing invalidates it.
Suppose a plan change grants the RBAC entitlement. Until the memo expires, each pod can still return False from is_enforced for up to 30 s. During that time _capability allows every member (RBAC_NOT_ENTITLED).
Use one of these fixes:
- Clear the memo entry from the subscription and plan writer.
- Record this bounded exception in
api/AGENTS.md, so the invariant stays accurate.
As per coding guidelines: "Only the principal record (fetch_principal), the membership (resolve_membership) and the project's organization are cached, and their writers invalidate them."
Source: Coding guidelines
| if is_active: | ||
| await recheck_trigger_principal( | ||
| project_id=project_id, trigger=existing, origin=RunOrigin.SUBSCRIPTION | ||
| ) | ||
|
|
There was a problem hiding this comment.
Resuming through a full-PUT edit skips the principal recheck.
set_subscription_active and set_schedule_active call recheck_trigger_principal before they set is_active=True. edit_subscription and edit_schedule do not. Both edit methods accept flags.is_active from the client. A client can therefore send is_active: true through the edit route on a trigger that was paused for access_denied or missing_principal. The trigger is then active again without a recheck, and the old meta.paused_reason can remain.
The dispatcher still refuses at the next fire, so access is not bypassed. The result is the delayed refusal that the recheck_trigger_principal docstring says it prevents, and the trigger pauses itself again. Run the same recheck in both edit methods when existing.flags.is_active is false and the edit sets it to true. When the recheck passes, also apply _resumed_meta.
Proposed fix (edit_schedule; apply the same change in edit_subscription with RunOrigin.SUBSCRIPTION)
-
if schedule.flags.is_active and not existing.flags.is_active: await recheck_trigger_principal( project_id=project_id, trigger=existing, origin=RunOrigin.SCHEDULE ) schedule.meta = _resumed_meta(schedule.meta)
| run_dbe.principal_kind = principal.kind.value | ||
| run_dbe.principal_id = principal.id | ||
|
|
There was a problem hiding this comment.
create_runs does not write principal_user_id, but create_run does.
create_run sets principal_kind, principal_id, and principal_user_id = user_id. edit_run also writes all three columns. create_runs writes only principal_kind and principal_id, so every run created in a batch has principal_user_id set to NULL.
EvaluationRunDBA documents principal_user_id as the acting user. For an installation principal, the run uses that user's role. A batch-created run with an installation principal then has no acting user when its principal is rechecked. Set the column the same way create_run does.
Proposed fix
-
run_dbe.principal_kind = principal.kind.value run_dbe.principal_id = principal.id + run_dbe.principal_user_id = user_id
|
|
||
| An API key now acts only in the project it belongs to. A request with the key to another project, such as `PATCH /projects/{id}` or `DELETE /projects/{id}`, returns `403`. Before, the key used its creator's role in the other project. Browser sessions do not change. | ||
|
|
||
| Workspace and organization management with an API key (invite, resend invite, role change, member removal, workspace update) needs a key of the workspace's default project. A key of another project in that workspace returns `403`. |
There was a problem hiding this comment.
List all organization-level writes that require a key from the default project.
This paragraph names only the workspace operations: invite, resend invite, role change, member removal and workspace update. The PR also applies the same rule to these writes:
- organization rename, ownership transfer and deletion
- workspace creation
- EE domain, SSO and billing writes
The acceptance test test_organization_writes_need_a_key_of_the_default_project asserts 403 for these writes. An operator who reads only this list may expect an automation that renames or transfers an organization with a non-default project key to keep working. That automation fails after the upgrade.
Proposed wording
-
--- "a/docs/docs/self-host/upgrade-and-migrate/01-upgrading.mdx" +++ "b/docs/docs/self-host/upgrade-and-migrate/01-upgrading.mdx" @@ -60,7 +60,7 @@ An API key now acts only in the project it belongs to. A request with the key to another project, such as `PATCH /projects/{id}` or `DELETE /projects/{id}`, returns `403`. Before, the key used its creator's role in the other project. Browser sessions do not change. -Workspace and organization management with an API key (invite, resend invite, role change, member removal, workspace update) needs a key of the workspace's default project. A key of another project in that workspace returns `403`. +Workspace and organization management with an API key needs a key of the default project. This covers invite, resend invite, role change, member removal and workspace update, and also organization rename, ownership transfer and deletion, and workspace creation. In Enterprise it also covers domain, SSO and billing writes. A key of another project returns `403`. ### Automations and evaluations run as the user or key that started them
Committable suggestion
-
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.Suggested changeWorkspace and organization management with an API key (invite, resend invite, role change, member removal, workspace update) needs a key of the workspace's default project. A key of another project in that workspace returns `403`. Workspace and organization management with an API key needs a key of the default project. This covers invite, resend invite, role change, member removal and workspace update, and also organization rename, ownership transfer and deletion, and workspace creation. In Enterprise it also covers domain, SSO and billing writes. A key of another project returns `403`.
Context
A run had no identity of its own. Every check and every minted credential used a bare user id: the request's user, or the
created_by_idof a stored row. So a schedule kept running as its creator after the creator lost access, an API key's run acted with the user's full reach in every project, an approval resumed under the approver's id, and a deleted key or a removed member could keep using a cached answer.check_action_accessand the credential exchange also cached their decisions.This PR is milestone M2 of the sharing program (design in #7747; stacked on M1, #7748): every run carries an explicit principal, and one service decides access on every call.
What changes for users
403 access_denied, never a 500.PATCHorDELETE /projects/{id}with a key of another project is now403. Before, the key used its creator's role in the other project. Browser sessions do not change.403). Browser sessions do not change.403 access_deniedor409 missing_principal) and clears the pause reason.meta.closed_reason.approved_by).403 access_denied. With a cold services cache the exchange refuses it before the run starts; with a cached exchange the run's first checked call (connection resolution) refuses it. Before, it failed with a 500.Changes
core/access:RunEnvelope(principal, authority, run context, sender, approver),PrincipalRefkindshuman,api_key,installation, andAccessService.decide(eligibility, restrictions, capability). The principal record and the membership are cached and invalidated by their writers; decisions are not.check_action_accesskeeps its signature and decides for that principal only: without request access, or for another user, it raises. Code without a request callscheck_humanordecide.sign_secret_tokenrequires it. The token's user is the run's acting user, derived from its authority, never an approver or a row's creator. A Secret without a principal is401 missing_principal.principal_kind,principal_id,sender_*,origin,origin_id,parent_run_idandapproved_by. Read DTOs expose the new fields; the TypeScript client is regenerated from an EE spec.403 access_deniedwith the API's reason, never a 500.MGET, and later decisions of the same request read nothing (request memo). The EE RBAC entitlement is a 30 second process memo. A warm checked request makes no more Redis round trips than M1.Before (a trigger tick):
After:
Docs
Updated in this PR (v0.124.0 ships M1 and M2 together, so the M1 doc changes are here too):
access_deniedagent run error.Migrations
oss000000046: nullable columns only (no default, no foreign key),ADD COLUMN IF NOT EXISTS, and two concurrent indexes on(project_id, artifact_id)of the trigger tables.lock_timeout5 s. Safe to rerun after an index failure.oss000000047(data): keyset batches of 1,000 on(project_id, id), 5 s lock and 60 s statement timeouts. Schedules, subscriptions and evaluation runs with a creator getprincipal = human(created_by_id); live triggers with no creator are paused (missing_principal, ids in the hook log);artifact_idis set for triggers bound to an agent directly (environment-bound triggers keepNULL). Idempotent.missing_envelopeon its next resume; a parked approval answered after the deploy returns409 missing_envelopeand is cancelled; a schedule of a deleted creator is paused at its next tick.Latency
Access-decision statements per request on warm caches (counted by phase with real DAOs;
test_run_principal_query_counts_postgres.py, OSS and EE):check_action_accessallow and denialGET /access/permissions/check(exchange)POST /gateways/credentialsA cold principal is one statement, a cold membership one statement.
Request-level latency (p50/p99 against the M1 base, both editions) is measured on the same local stacks; the table follows in a comment.
Tests / notes
/mand/w: web chat attribution per sender, API keys in and out of their project, run credentials, deleted keys, schedules (fire, deleted creator, deleted key, archive), evaluations (start, rerun, principal rules), approvals from the UI, channel installation turns, a settings sweep, and log checks (no credential strings, ids only in denial lines). QA found seven defects: the session queue stopped after a refused run, the approver was missing on the resumed turn, a removed member's invoke failed with a 500 inside the run, and four small trigger and evaluation messages. All are fixed in this PR with tests.Release notes
Human steps
oss000000047docstring on a copy of each region's database, then both revisions, and time the backfill. Keep the counts in the private release notes.Merge note: revision ids
The central allocation is M1
044/045, M2046/047, M3048/049, M4a050. Whichever of M2 and a later milestone merges second re-points its first revision to the other's head.Follow-ups (not in this PR)
401for that project (the M1 rule that a non-member session is unauthenticated there). A clear403 access_deniedneeds frontend handling.