fix(authz): grant legacy agent register principal by rpatel-scale · Pull Request #325 · scaleapi/scale-agentex

rpatel-scale · 2026-06-18T20:30:04Z

Summary

Restore legacy /agents/register compatibility by accepting principal_context in the register request body.
Use the request-state principal when present, otherwise fall back to the body principal for create-check and ownership grant.
Keep the fix(authz): skip create-check/grant on unauthenticated agent self-register #292 behavior where register skips authz when no resolvable principal is provided, avoiding the unauthenticated self-register crashloop.
Regenerate the Agentex OpenAPI spec.

Validation

uv run pytest agentex/tests/unit/api/test_agents_authz.py -q
uv run ruff check agentex/src/api/routes/agents.py agentex/src/api/schemas/agents.py agentex/tests/unit/api/test_agents_authz.py
uv run ruff format --check agentex/src/api/routes/agents.py agentex/src/api/schemas/agents.py agentex/tests/unit/api/test_agents_authz.py
make gen-openapi

Greptile Summary

Overview

This PR restores legacy /agents/register compatibility by accepting principal_context in the request body and falling back to it when the request-state principal (set by auth middleware) is not resolvable. The new principal_context: Any | None field on RegisterAgentRequest is forwarded directly to authorization_service.check(...) and authorization_service.grant(...) when the endpoint is hit without an authenticated session.

Key Changes

RegisterAgentRequest gains principal_context: Any | None (defaults to None), exposed publicly in the OpenAPI schema.
register_agent route logic: if authorization_service.principal_context is unresolvable (no user_id/service_account_id), falls back to request.principal_context from the body. Both check and grant now receive an explicit principal_context= kwarg.
New test (test_body_principal_enforces_check_and_grant) verifies the body principal is forwarded to both check and grant.

Issues Found

Trust Boundary — Body principal injection on whitelisted endpoint

/agents/register is in WHITELISTED_ROUTES; the auth middleware sets request.state.principal_context = None and performs no authentication. Any external caller can now POST to this endpoint with an arbitrary principal_context: {"user_id": "victim-id"} in the body. When the request-state principal is None, the code falls back to this body-supplied dict, _has_resolvable_creator returns True, and authorization_service.grant(...) is called forwarding the attacker-controlled dict to agentex-auth /v1/authz/grant with no cryptographic proof that the caller is that identity. Whether exploitation succeeds depends entirely on whether agentex-auth validates the principal independently.

Missing test coverage for priority logic

Unresolvable non-None auth-service principal + resolvable body principal: test_unresolvable_creator_skips_check_and_grant always uses _request() (body=None). The case auth_service.principal={"account_id":"acct"} + body={"user_id":"u"} — where body fallback triggers enforce_ownership=True — is not tested.
Request-state principal takes precedence over body when both are resolvable: No test verifies that a resolvable request-state principal prevents the body principal from being used.

Existing tests don't assert `principal_context` kwarg on the request-state principal path

test_dict_principal_enforces_check_and_grant and test_object_principal_enforces_check_and_grant assert check/grant were called once but do not inspect await_args.kwargs["principal_context"]. A regression where the wrong principal is passed would not be caught, unlike the new body-principal test which does check kwargs.

Confidence Score: 3/5

Functionally correct for the intended legacy case, but the trust model relies on network isolation with no code-level guard, and three meaningful test coverage gaps exist in the priority/fallback logic.

The core fallback logic is correct and the new test covers the primary intended case. However: (1) the security trust boundary is fully open at the code layer — any caller who can reach the whitelisted endpoint can forge any principal; (2) two critical test gaps leave the priority ordering (request-state > body, and unresolvable-state + resolvable-body) completely uncovered; (3) the existing request-state tests don't assert the correct principal_context kwarg after the API change. These are real risks to correctness and security, not theoretical ones.

agentex/src/api/routes/agents.py (lines 199-201, trust boundary), agentex/tests/unit/api/test_agents_authz.py (missing priority and fallback test cases), agentex/src/api/schemas/agents.py (Any type too permissive)

Security Review

Unauthenticated principal injection (agentex/src/api/routes/agents.py:199-200, agentex/src/api/schemas/agents.py:91-93): /agents/register is whitelisted so authentication is skipped for all callers. The new principal_context: Any | None body field means any external caller can supply {"user_id": "arbitrary-id"}. The code falls back to this dict when the request-state principal is None, calls authorization_service.check(...) and authorization_service.grant(...) with the caller-supplied identity, and forwards it directly to agentex-auth without any cryptographic validation at the route layer. Trust relies entirely on network isolation of the endpoint.

Important Files Changed

Filename	Overview
agentex/src/api/routes/agents.py	Adds body-principal fallback in register_agent: when auth-service principal is unresolvable, falls back to request.principal_context from the body and forwards it explicitly to check/grant. The fallback logic is correct, but the trust boundary is fully open — any unauthenticated caller to the whitelisted endpoint can supply an arbitrary principal dict.
agentex/src/api/schemas/agents.py	Adds principal_context: Any
agentex/tests/unit/api/test_agents_authz.py	Adds test_body_principal_enforces_check_and_grant and updates _request helper. New test correctly asserts principal_context kwargs. Missing: (1) test for unresolvable-non-None auth-service principal + resolvable body principal, (2) test for request-state precedence over body when both are resolvable, (3) existing enforce tests don't assert principal_context kwarg.
agentex/openapi.yaml	Regenerated spec adds principal_context field. Uses anyOf: [{}] (any JSON value) which is technically correct for Any

Comments Outside Diff (2)

agentex/tests/unit/api/test_agents_authz.py, line 432-440 (link)

Missing test: request-state principal takes precedence over body principal

There is no test for the case where both the request-state principal (resolvable, e.g. {"user_id":"req-user"}) and the body principal_context (e.g. {"user_id":"body-user"}) are present. The request-state principal should win and the body should be silently ignored.

Without a test, a future refactor that accidentally reverses the precedence (falling back to body even when the request-state is resolvable) would pass all current tests. This is a straightforward case to add:

async def test_request_state_principal_takes_precedence_over_body(self):
    req_principal = {"user_id": "req-user", "account_id": "acct"}
    body_principal = {"user_id": "body-user", "account_id": "acct"}
    authz, use_case, api_keys = self._mocks(principal_context=req_principal)

    await register_agent(
        self._request(principal_context=body_principal), use_case, authz, api_keys
    )

    authz.check.assert_awaited_once()
    assert authz.check.await_args.kwargs["principal_context"] == req_principal  # not body_principal
    authz.grant.assert_awaited_once()
    assert authz.grant.await_args.kwargs["principal_context"] == req_principal

Prompt To Fix With AI

This is a comment left during a code review.
Path: agentex/tests/unit/api/test_agents_authz.py
Line: 432-440

Comment:
**Missing test: request-state principal takes precedence over body principal**

There is no test for the case where **both** the request-state principal (resolvable, e.g. `{"user_id":"req-user"}`) **and** the body `principal_context` (e.g. `{"user_id":"body-user"}`) are present. The request-state principal should win and the body should be silently ignored.

Without a test, a future refactor that accidentally reverses the precedence (falling back to body even when the request-state is resolvable) would pass all current tests. This is a straightforward case to add:

```python
async def test_request_state_principal_takes_precedence_over_body(self):
    req_principal = {"user_id": "req-user", "account_id": "acct"}
    body_principal = {"user_id": "body-user", "account_id": "acct"}
    authz, use_case, api_keys = self._mocks(principal_context=req_principal)

    await register_agent(
        self._request(principal_context=body_principal), use_case, authz, api_keys
    )

    authz.check.assert_awaited_once()
    assert authz.check.await_args.kwargs["principal_context"] == req_principal  # not body_principal
    authz.grant.assert_awaited_once()
    assert authz.grant.await_args.kwargs["principal_context"] == req_principal
```

How can I resolve this? If you propose a fix, please make it concise.

agentex/tests/unit/api/test_agents_authz.py, line 432-453 (link)

Existing tests don't assert principal_context kwarg for the request-state principal path

Both test_dict_principal_enforces_check_and_grant and test_object_principal_enforces_check_and_grant only check assert_awaited_once() without inspecting await_args.kwargs["principal_context"]. Now that the route explicitly passes principal_context=principal_context as a kwarg (new in this PR), the test should verify the correct principal is forwarded — especially since a bug where the wrong principal was passed (e.g., None or the body principal) would be invisible to the current assertions.

The new test_body_principal_enforces_check_and_grant correctly asserts await_args.kwargs["principal_context"] == body_principal. The same pattern should be applied to these two existing tests:

# In test_dict_principal_enforces_check_and_grant:
assert authz.check.await_args.kwargs["principal_context"] == {"user_id": "u", "account_id": "acct"}
assert authz.grant.await_args.kwargs["principal_context"] == {"user_id": "u", "account_id": "acct"}

# In test_object_principal_enforces_check_and_grant:
expected = SimpleNamespace(user_id="u", service_account_id=None, account_id="acct")
assert authz.check.await_args.kwargs["principal_context"] == expected
assert authz.grant.await_args.kwargs["principal_context"] == expected

Prompt To Fix With AI

This is a comment left during a code review.
Path: agentex/tests/unit/api/test_agents_authz.py
Line: 432-453

Comment:
**Existing tests don't assert `principal_context` kwarg for the request-state principal path**

Both `test_dict_principal_enforces_check_and_grant` and `test_object_principal_enforces_check_and_grant` only check `assert_awaited_once()` without inspecting `await_args.kwargs["principal_context"]`. Now that the route explicitly passes `principal_context=principal_context` as a kwarg (new in this PR), the test should verify the correct principal is forwarded — especially since a bug where the wrong principal was passed (e.g., `None` or the body principal) would be invisible to the current assertions.

The new `test_body_principal_enforces_check_and_grant` correctly asserts `await_args.kwargs["principal_context"] == body_principal`. The same pattern should be applied to these two existing tests:

```python
# In test_dict_principal_enforces_check_and_grant:
assert authz.check.await_args.kwargs["principal_context"] == {"user_id": "u", "account_id": "acct"}
assert authz.grant.await_args.kwargs["principal_context"] == {"user_id": "u", "account_id": "acct"}

# In test_object_principal_enforces_check_and_grant:
expected = SimpleNamespace(user_id="u", service_account_id=None, account_id="acct")
assert authz.check.await_args.kwargs["principal_context"] == expected
assert authz.grant.await_args.kwargs["principal_context"] == expected
```

How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 6 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 6
agentex/src/api/routes/agents.py:199-201
**Security / trust-boundary concern: unauthenticated body principal injection**

`/agents/register` is in `WHITELISTED_ROUTES`, so the auth middleware sets `request.state.principal_context = None` and skips all credential validation for every request to this path — including requests from external, untrusted callers.

With this PR, any caller to the endpoint can supply:
```json
{"principal_context": {"user_id": "any-user-id", "account_id": "any-account"}}
```
`_has_resolvable_creator` will return `True` for that dict, `enforce_ownership` becomes `True`, and the code proceeds to call `authorization_service.grant(AgentexResource.agent(...), principal_context=<attacker_dict>)`. The `AgentexAuthorizationProxy.grant` then forwards this dict verbatim to `agentex-auth /v1/authz/grant` with no cryptographic proof that the caller is actually that user.

Whether this results in a successful ownership escalation depends on whether agentex-auth performs independent principal validation server-side. If it accepts the principal dict at face value (as a trusted internal call), any caller who can reach this endpoint can claim ownership of agents under any identity.

**Recommendation:** If the body-principal trust model depends on network isolation (only internal pods can reach this endpoint), document that assumption explicitly. Consider adding a comment on the `principal_context` field and here explaining the trust model, and/or adding a check that explicitly limits this fallback to only when `AGENTEX_AUTH_URL` is configured in a way that signals internal-only trust.

### Issue 2 of 6
agentex/tests/unit/api/test_agents_authz.py:404-415
**Missing test: unresolvable non-None auth-service principal + resolvable body principal**

The parametrize covers `[None, {}, {"account_id":"acct"}]` for the auth-service principal, but always calls `self._request()` with no body `principal_context`. This means the case:

- auth service has `{"account_id": "acct"}` (unresolvable) **AND** body has `{"user_id": "u"}` (resolvable)

is never tested. With the new fallback logic, the body principal would be used here (`enforce_ownership=True`), which is the intended behavior — but there is zero coverage for it. A regression that skips enforcement in this case would pass all current tests.

Consider adding a parametrized variant like:
```python
@pytest.mark.parametrize(
    "auth_principal,body_principal",
    [
        ({}, {"user_id": "u", "account_id": "acct"}),
        ({"account_id": "acct"}, {"user_id": "u", "account_id": "acct"}),
    ],
)
async def test_unresolvable_auth_service_with_resolvable_body_enforces(self, auth_principal, body_principal):
    authz, use_case, api_keys = self._mocks(principal_context=auth_principal)
    await register_agent(self._request(principal_context=body_principal), use_case, authz, api_keys)
    authz.check.assert_awaited_once()
    assert authz.check.await_args.kwargs["principal_context"] == body_principal
    authz.grant.assert_awaited_once()
    assert authz.grant.await_args.kwargs["principal_context"] == body_principal
```

### Issue 3 of 6
agentex/tests/unit/api/test_agents_authz.py:432-440
**Missing test: request-state principal takes precedence over body principal**

There is no test for the case where **both** the request-state principal (resolvable, e.g. `{"user_id":"req-user"}`) **and** the body `principal_context` (e.g. `{"user_id":"body-user"}`) are present. The request-state principal should win and the body should be silently ignored.

Without a test, a future refactor that accidentally reverses the precedence (falling back to body even when the request-state is resolvable) would pass all current tests. This is a straightforward case to add:

```python
async def test_request_state_principal_takes_precedence_over_body(self):
    req_principal = {"user_id": "req-user", "account_id": "acct"}
    body_principal = {"user_id": "body-user", "account_id": "acct"}
    authz, use_case, api_keys = self._mocks(principal_context=req_principal)

    await register_agent(
        self._request(principal_context=body_principal), use_case, authz, api_keys
    )

    authz.check.assert_awaited_once()
    assert authz.check.await_args.kwargs["principal_context"] == req_principal  # not body_principal
    authz.grant.assert_awaited_once()
    assert authz.grant.await_args.kwargs["principal_context"] == req_principal
```

### Issue 4 of 6
agentex/tests/unit/api/test_agents_authz.py:432-453
**Existing tests don't assert `principal_context` kwarg for the request-state principal path**

Both `test_dict_principal_enforces_check_and_grant` and `test_object_principal_enforces_check_and_grant` only check `assert_awaited_once()` without inspecting `await_args.kwargs["principal_context"]`. Now that the route explicitly passes `principal_context=principal_context` as a kwarg (new in this PR), the test should verify the correct principal is forwarded — especially since a bug where the wrong principal was passed (e.g., `None` or the body principal) would be invisible to the current assertions.

The new `test_body_principal_enforces_check_and_grant` correctly asserts `await_args.kwargs["principal_context"] == body_principal`. The same pattern should be applied to these two existing tests:

```python
# In test_dict_principal_enforces_check_and_grant:
assert authz.check.await_args.kwargs["principal_context"] == {"user_id": "u", "account_id": "acct"}
assert authz.grant.await_args.kwargs["principal_context"] == {"user_id": "u", "account_id": "acct"}

# In test_object_principal_enforces_check_and_grant:
expected = SimpleNamespace(user_id="u", service_account_id=None, account_id="acct")
assert authz.check.await_args.kwargs["principal_context"] == expected
assert authz.grant.await_args.kwargs["principal_context"] == expected
```

### Issue 5 of 6
agentex/src/api/schemas/agents.py:91-93
**`principal_context: Any | None` accepts any JSON value without structural validation**

The `Any | None` type passes no shape constraint to either Pydantic or the OpenAPI schema. This means callers can supply a string, a list, or a deeply-nested object — none of which are meaningful principals. While `_has_resolvable_creator` safely returns `False` for non-dict/non-object values (so enforcement is skipped), the field still accepts arbitrary input.

Consider narrowing to `dict[str, Any] | None` to at minimum enforce that the principal is a JSON object, which matches the actual expected shape and generates a cleaner OpenAPI schema:

```python
principal_context: dict[str, Any] | None = Field(
    default=None,
    description=(
        "Principal used for authorization. "
        "Expected keys: 'user_id' or 'service_account_id'. "
        "Only honoured when the endpoint has no request-state principal "
        "(legacy whitelisted self-registration)."
    ),
)
```

This also makes the intent and expected format discoverable from the schema alone.

### Issue 6 of 6
agentex/openapi.yaml:5222-5229
**`anyOf: [{}]` is maximally permissive and the description omits expected structure**

```yaml
principal_context:
  anyOf:
  - {}
  - type: 'null'
  title: Principal Context
  description: Principal used for authorization
```

`{}` in JSON Schema means "validates anything" — any string, number, list, or object will pass. The description says only "Principal used for authorization" without naming expected keys (`user_id` / `service_account_id`). API consumers reading the spec will not know what to supply.

If the schema is narrowed to `dict[str, Any] | None` in the Python model (see comment on `agents.py`), the generated spec would become:
```yaml
principal_context:
  anyOf:
  - type: object
    additionalProperties: true
  - type: 'null'
```
which is significantly more descriptive and filters out non-object values at the serialization layer.

_{Reviews (1): Last reviewed commit: "fix(authz): grant legacy agent register ..." | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

github-actions · 2026-06-18T20:30:49Z

✱ Stainless preview builds

This PR will update the agentex-sdk SDKs with the following commit messages.

openapi

feat(api): add principal_context field to agent model

python

chore(internal): regenerate SDK with no functional changes

typescript

chore(internal): regenerate SDK with no functional changes

✅ agentex-sdk-openapi studio · code

Your SDK build had at least one "note" diagnostic.
generate ✅

⚠️

agentex-sdk-typescript studio · code

Your SDK build had at least one "warning" diagnostic.
generate ⚠️ → build ⏭️ → lint ⏭️ → test ✅

⚠️

agentex-sdk-python studio · code

Your SDK build had at least one "warning" diagnostic.
generate ⚠️ → build ⏭️ → lint ⏭️ → test ✅

This comment is auto-generated by GitHub Actions and is automatically kept up to date as you push.
If you push custom code to the preview branch, re-run this workflow to update the comment.
Last updated: 2026-06-18 20:47:37 UTC

greptile-apps · 2026-06-18T20:54:04Z

+    if not _has_resolvable_creator(principal_context):
+        principal_context = request.principal_context
+    enforce_ownership = _has_resolvable_creator(principal_context)


Security / trust-boundary concern: unauthenticated body principal injection

/agents/register is in WHITELISTED_ROUTES, so the auth middleware sets request.state.principal_context = None and skips all credential validation for every request to this path — including requests from external, untrusted callers.

With this PR, any caller to the endpoint can supply:

{"principal_context": {"user_id": "any-user-id", "account_id": "any-account"}}

_has_resolvable_creator will return True for that dict, enforce_ownership becomes True, and the code proceeds to call authorization_service.grant(AgentexResource.agent(...), principal_context=<attacker_dict>). The AgentexAuthorizationProxy.grant then forwards this dict verbatim to agentex-auth /v1/authz/grant with no cryptographic proof that the caller is actually that user.

Whether this results in a successful ownership escalation depends on whether agentex-auth performs independent principal validation server-side. If it accepts the principal dict at face value (as a trusted internal call), any caller who can reach this endpoint can claim ownership of agents under any identity.

Recommendation: If the body-principal trust model depends on network isolation (only internal pods can reach this endpoint), document that assumption explicitly. Consider adding a comment on the principal_context field and here explaining the trust model, and/or adding a check that explicitly limits this fallback to only when AGENTEX_AUTH_URL is configured in a way that signals internal-only trust.

Prompt To Fix With AI

This is a comment left during a code review. Path: agentex/src/api/routes/agents.py Line: 199-201 Comment: **Security / trust-boundary concern: unauthenticated body principal injection** `/agents/register` is in `WHITELISTED_ROUTES`, so the auth middleware sets `request.state.principal_context = None` and skips all credential validation for every request to this path — including requests from external, untrusted callers. With this PR, any caller to the endpoint can supply: ```json {"principal_context": {"user_id": "any-user-id", "account_id": "any-account"}} ``` `_has_resolvable_creator` will return `True` for that dict, `enforce_ownership` becomes `True`, and the code proceeds to call `authorization_service.grant(AgentexResource.agent(...), principal_context=<attacker_dict>)`. The `AgentexAuthorizationProxy.grant` then forwards this dict verbatim to `agentex-auth /v1/authz/grant` with no cryptographic proof that the caller is actually that user. Whether this results in a successful ownership escalation depends on whether agentex-auth performs independent principal validation server-side. If it accepts the principal dict at face value (as a trusted internal call), any caller who can reach this endpoint can claim ownership of agents under any identity. **Recommendation:** If the body-principal trust model depends on network isolation (only internal pods can reach this endpoint), document that assumption explicitly. Consider adding a comment on the `principal_context` field and here explaining the trust model, and/or adding a check that explicitly limits this fallback to only when `AGENTEX_AUTH_URL` is configured in a way that signals internal-only trust. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-06-18T20:54:05Z

@@ -413,6 +414,21 @@ async def test_unresolvable_creator_skips_check_and_grant(self, principal_contex
        use_case.register_agent.assert_awaited_once()
        assert resp.agent_api_key == "internal-key"


Missing test: unresolvable non-None auth-service principal + resolvable body principal

The parametrize covers [None, {}, {"account_id":"acct"}] for the auth-service principal, but always calls self._request() with no body principal_context. This means the case:

auth service has {"account_id": "acct"} (unresolvable) AND body has {"user_id": "u"} (resolvable)

is never tested. With the new fallback logic, the body principal would be used here (enforce_ownership=True), which is the intended behavior — but there is zero coverage for it. A regression that skips enforcement in this case would pass all current tests.

Consider adding a parametrized variant like:

@pytest.mark.parametrize( "auth_principal,body_principal", [ ({}, {"user_id": "u", "account_id": "acct"}), ({"account_id": "acct"}, {"user_id": "u", "account_id": "acct"}), ], ) async def test_unresolvable_auth_service_with_resolvable_body_enforces(self, auth_principal, body_principal): authz, use_case, api_keys = self._mocks(principal_context=auth_principal) await register_agent(self._request(principal_context=body_principal), use_case, authz, api_keys) authz.check.assert_awaited_once() assert authz.check.await_args.kwargs["principal_context"] == body_principal authz.grant.assert_awaited_once() assert authz.grant.await_args.kwargs["principal_context"] == body_principal

Prompt To Fix With AI

This is a comment left during a code review. Path: agentex/tests/unit/api/test_agents_authz.py Line: 404-415 Comment: **Missing test: unresolvable non-None auth-service principal + resolvable body principal** The parametrize covers `[None, {}, {"account_id":"acct"}]` for the auth-service principal, but always calls `self._request()` with no body `principal_context`. This means the case: - auth service has `{"account_id": "acct"}` (unresolvable) **AND** body has `{"user_id": "u"}` (resolvable) is never tested. With the new fallback logic, the body principal would be used here (`enforce_ownership=True`), which is the intended behavior — but there is zero coverage for it. A regression that skips enforcement in this case would pass all current tests. Consider adding a parametrized variant like: ```python @pytest.mark.parametrize( "auth_principal,body_principal", [ ({}, {"user_id": "u", "account_id": "acct"}), ({"account_id": "acct"}, {"user_id": "u", "account_id": "acct"}), ], ) async def test_unresolvable_auth_service_with_resolvable_body_enforces(self, auth_principal, body_principal): authz, use_case, api_keys = self._mocks(principal_context=auth_principal) await register_agent(self._request(principal_context=body_principal), use_case, authz, api_keys) authz.check.assert_awaited_once() assert authz.check.await_args.kwargs["principal_context"] == body_principal authz.grant.assert_awaited_once() assert authz.grant.await_args.kwargs["principal_context"] == body_principal ``` How can I resolve this? If you propose a fix, please make it concise.

## Summary The whitelisted `/agents/register` path clears the middleware principal. The pod SDK ships the manifest-declared identity in the request body, but `_register_in_auth` was still reading only from the middleware and silently skipping ownership registration. This threads the body-supplied principal into the use case as a fallback, mirroring what #325 (`cae5f94`) already did for the route's `check`/`grant` fallback. Symptom pre-fix: agent registration succeeds, no error logs, WARN `Skipping authorization registration for agent: no creator resolvable` fires at `agents_use_case.py:83`, and the agent has no owner tuple in `agentex_permissions` — invisible to the SGP UI listing. ## Root cause Bisected across three commits on the same code path: | Commit | Date | What it did | |---|---|---| | `14796e9` (#270) | Jun 3 | Moved ownership registration into `_register_in_auth`, keyed off `self.authorization_service.principal_context`. | | `63f89e7` (#292) | Jun 9 | Route-layer `_has_resolvable_creator` guard — skips `check`/`grant` when middleware principal is missing (i.e. the whitelisted pod path). | | `cae5f94` (#325) | Jun 18 | Route-layer fallback to the body's `principal_context` for `check`/`grant`. | `cae5f94` recovered ownership grants for authenticated CLI/UI callers via the route layer, but the use case's `register_resource` call inside `_register_in_auth` was still reading only from the middleware. Result: every pod-based deploy since Jun 3 that lands on the whitelisted route lost its owner tuple. ## What this change does 1. `agentex/src/domain/use_cases/agents_use_case.py` - `_register_in_auth` accepts an optional `body_principal_context` and uses it as fallback when the middleware principal isn't resolvable. Passes the resolved principal explicitly to `register_resource(..., principal_context=...)` so it doesn't silently default to the (None) middleware principal. - Extracts the dict-vs-object identity check into a private `_has_resolvable_creator` staticmethod (same shape as the one in `agents.py:53`). - `register_agent` accepts and forwards `body_principal_context`. 2. `agentex/src/api/routes/agents.py` - Route passes `body_principal_context=request.principal_context` when calling the use case, matching the precedence already applied to `check`/`grant`. 3. `agentex/tests/integration/use_cases/test_agent_authz_dual_write.py` - `test_create_falls_back_to_body_principal_when_middleware_missing` — covers the whitelisted pod path with a body-supplied service account. - `test_middleware_principal_takes_precedence_over_body` — regression guard confirming authenticated CLI/UI callers keep using the middleware identity. ## What this change does NOT do - `register_build` is intentionally left alone. That route isn't whitelisted, so the middleware principal is always set for legitimate callers — there's no trigger for the fallback there. Keeping the fix scoped to the actual regression. - `_has_resolvable_creator` is now duplicated between `agents.py` (route) and `agents_use_case.py` (use case). De-duping into a shared util would be an unrelated refactor and would need to sit somewhere neutral (not in `api/`, since the domain shouldn't import from it). Happy to do that in a follow-up if a reviewer prefers. ## Symptom (from Rocket beta) <img width="3024" height="1492" alt="image" src="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/user-attachments/assets/ee5c0211-96d1-43f3-a191-2b2fa64294ae" /> The manifest for this agent declared a valid service account: ```yaml auth: principal: service_account_id: {{SERVICE_ACCOUNT_ID}} account_id: {{ACCOUNT_ID}} ``` The SDK correctly base64-encodes this into `AUTH_PRINCIPAL_B64` and ships it in the register body's `principal_context` field (see `scale-agentex-python/src/agentex/lib/utils/registration.py`). The service was just not reading it in `_register_in_auth`. Manual backfill of the `agentex_permissions` row (via `POST /private/v5/agentex/permissions` on egp-api-backend) restored visibility, confirming the SGP permissions row is what was missing. ## Test plan - [x] New: `test_create_falls_back_to_body_principal_when_middleware_missing` - [x] New: `test_middleware_principal_takes_precedence_over_body` - [x] Existing 7 Mock-based tests in `TestAgentRegisterOnCreate` still pass locally (9/9 passed, 6 Docker-only tests deselected) - [ ] Verify on beta: deploy a fresh agent from a Rocket manifest, confirm the `agentex_permissions` row is written automatically (no manual backfill required) and the agent shows up in the SGP UI listing  <h3>Greptile Summary</h3> This PR fixes a missing body-principal fallback in `_register_in_auth` that caused pod self-registrations on the whitelisted `/agents/register` path to silently skip the `agentex_permissions` ownership write, leaving agents invisible to the SGP UI. It mirrors the same fallback that commit `cae5f94` (#325) already applied to the route-layer `check`/`grant` calls. - `_register_in_auth` now falls back to `body_principal_context` when `authorization_service.principal_context` is unresolvable, and passes the resolved principal explicitly to `register_resource` so the middleware-None path is no longer silently bypassed. - `_safe_deregister` was updated to accept an explicit `principal_context` (defaulting to the `...` Ellipsis sentinel that `AuthorizationService` already uses to mean "use middleware") so register and deregister remain symmetric on all paths, including duplicate-race compensation. - Three new integration tests cover the body-principal fallback, compensation symmetry on the whitelisted path, and middleware-principal precedence. <details><summary><h3>Confidence Score: 5/5</h3></summary> - Safe to merge. The change is narrowly scoped to the whitelisted pod registration path and does not affect any authenticated caller flows. - The root cause is clearly identified and the fix is minimal: a two-level fallback (`middleware → body → skip`) that mirrors a pattern already validated in the route layer. The `...` Ellipsis sentinel is correctly propagated as the default for `_safe_deregister`, preserving the middleware-principal behavior on the delete path. All three compensation branches (register failure, persist failure, duplicate race) are symmetric with the resolved principal, and the new integration tests directly exercise the whitelisted-path scenario, the compensation case, and the precedence rule. - No files require special attention. </details> <details><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | agentex/src/domain/use_cases/agents_use_case.py | Core fix: `_register_in_auth` now falls back to `body_principal_context` when the middleware principal is unresolvable, and passes the resolved principal explicitly to `register_resource`; `_safe_deregister` correctly uses `...` as its default sentinel; `_has_resolvable_creator` extracted as a clean static method. All compensation paths (`DuplicateItemError` and general `Exception`) are symmetric with the new resolved principal. | | agentex/src/api/routes/agents.py | One-line addition: forwards `body_principal_context=request.principal_context` to the use case, mirroring the same fallback already applied to `check`/`grant`. Minimal and correct. | | agentex/tests/integration/use_cases/test_agent_authz_dual_write.py | Three new tests cover the whitelisted-path fallback, register/deregister symmetry with body principal on duplicate race, and middleware-principal precedence. The `_record_existence` side-effect correctly gains `*, principal_context=None` to match the new kwarg. Existing tests remain unaffected. | </details> <details><summary><h3>Sequence Diagram</h3></summary> ```mermaid sequenceDiagram participant SDK as Pod SDK participant Route as agents.py (route) participant UC as AgentsUseCase participant Auth as AuthorizationService Note over Route: middleware_principal = None (whitelisted path) SDK->>Route: "POST /agents/register (body: principal_context={service_account_id:…})" Route->>Route: _has_resolvable_creator(middleware_principal) → False Route->>Route: "principal_context = request.principal_context (body fallback)" Route->>Route: "enforce_ownership = True" Route->>Auth: "check(agent:*, CREATE, principal_context=body_principal)" Route->>UC: "register_agent(…, body_principal_context=body_principal)" UC->>UC: _register_in_auth(agent_id, body_principal_context) UC->>UC: _has_resolvable_creator(middleware_principal) → False UC->>UC: "principal_context = body_principal_context" UC->>Auth: "register_resource(agent:id, principal_context=body_principal)" UC-->>Route: "agent_entity, resolved_principal=body_principal" Route->>Auth: "grant(agent:id, principal_context=body_principal)" Route-->>SDK: RegisterAgentResponse Note over UC: On DuplicateItemError compensation: UC->>Auth: "deregister_resource(agent:id, principal_context=resolved_principal)" ``` </details> <sub>Reviews (8): Last reviewed commit: ["Merge branch 'main' into akam/fix-authz-..."](17a4dc4) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=46529087)</sub>

fix(authz): grant legacy agent register principal

cfcc63d

rpatel-scale marked this pull request as ready for review June 18, 2026 20:35

rpatel-scale requested a review from a team as a code owner June 18, 2026 20:35

rpatel-scale enabled auto-merge (squash) June 18, 2026 20:35

rpatel-scale requested a review from smoreinis June 18, 2026 20:44

smoreinis approved these changes Jun 18, 2026

View reviewed changes

rpatel-scale merged commit cae5f94 into main Jun 18, 2026
30 of 31 checks passed

rpatel-scale deleted the codex/legacy-register-principal-grant branch June 18, 2026 20:45

greptile-apps Bot reviewed Jun 18, 2026

View reviewed changes

github-actions Bot mentioned this pull request Jun 19, 2026

chore(main): release 0.2.0 #326

Open

alvinkam2001 mentioned this pull request Jul 23, 2026

fix(authz): fall back to body principal in _register_in_auth #373

Merged

4 tasks

Navigation Menu

Uh oh!

fix(authz): grant legacy agent register principal - #325

fix(authz): grant legacy agent register principal#325
rpatel-scale merged 1 commit into
mainfrom
codex/legacy-register-principal-grant

rpatel-scale commented Jun 18, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

github-actions Bot commented Jun 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

greptile-apps Bot Jun 18, 2026

Uh oh!

greptile-apps Bot Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -413,6 +414,21 @@ async def test_unresolvable_creator_skips_check_and_grant(self, principal_contex
		use_case.register_agent.assert_awaited_once()
		assert resp.agent_api_key == "internal-key"

Uh oh!

Conversation

rpatel-scale commented Jun 18, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Greptile Summary

Overview

Key Changes

Issues Found

Trust Boundary — Body principal injection on whitelisted endpoint

Missing test coverage for priority logic

Existing tests don't assert principal_context kwarg on the request-state principal path

Confidence Score: 3/5

Security Review

Important Files Changed

Comments Outside Diff (2)

Uh oh!

github-actions Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✱ Stainless preview builds

Uh oh!

Uh oh!

greptile-apps Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rpatel-scale commented Jun 18, 2026 •

edited by greptile-apps Bot

Loading

Existing tests don't assert `principal_context` kwarg on the request-state principal path

github-actions Bot commented Jun 18, 2026 •

edited

Loading