fix(agents): scope agent deployments to creator via on-behalf-of - #1000
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR propagates ChangesDelegated authorization
Sequence Diagram(s)sequenceDiagram
participant AgentDeploymentController
participant DeploymentsRunnerBackend
participant auth_proxy
participant InferenceGateway
participant AuthClient
AgentDeploymentController->>DeploymentsRunnerBackend: Create deployment with created_by
DeploymentsRunnerBackend->>auth_proxy: Configure delegated principal
auth_proxy->>InferenceGateway: Forward request with on-behalf-of header
InferenceGateway->>AuthClient: Check workspace permission
AuthClient-->>InferenceGateway: Allow or deny
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/nmp_common/src/nmp/common/auth/workload_proxy/main.py (1)
64-70: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winStrip the OBO email header too.
X-NMP-Principal-On-Behalf-Of-Emailsurvives forwarding. A colocated workload can attach untrusted delegated identity metadata to the proxy-stamped OBO principal. Strip it alongside the OBO ID header and cover both configured/unconfigured paths.Proposed fix
_ON_BEHALF_OF_HEADER = "x-nmp-principal-on-behalf-of" +_ON_BEHALF_OF_EMAIL_HEADER = "x-nmp-principal-on-behalf-of-email" _PRINCIPAL_ID_HEADER, _ON_BEHALF_OF_HEADER, + _ON_BEHALF_OF_EMAIL_HEADER,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nmp_common/src/nmp/common/auth/workload_proxy/main.py` around lines 64 - 70, Update _STRIP_REQUEST_HEADERS to include the X-NMP-Principal-On-Behalf-Of-Email header alongside _ON_BEHALF_OF_HEADER. Ensure request forwarding strips this email header in both configured and unconfigured proxy paths, and add or update coverage for both cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/nmp_common/src/nmp/common/auth/workload_proxy/main.py`:
- Around line 165-172: Update the startup log in the auth-proxy sidecar around
the logger.info call to remove the on_behalf_of value entirely and report only
whether delegation is enabled. Preserve the other startup context and ensure no
delegated principal or creator identifier is written to logs.
---
Outside diff comments:
In `@packages/nmp_common/src/nmp/common/auth/workload_proxy/main.py`:
- Around line 64-70: Update _STRIP_REQUEST_HEADERS to include the
X-NMP-Principal-On-Behalf-Of-Email header alongside _ON_BEHALF_OF_HEADER. Ensure
request forwarding strips this email header in both configured and unconfigured
proxy paths, and add or update coverage for both cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4ea24ed1-6a81-45a1-8799-b77f6629266d
📒 Files selected for processing (17)
packages/nmp_common/src/nmp/common/auth/workload_proxy/main.pypackages/nmp_common/tests/auth/test_workload_proxy.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/backend.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/controller.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/in_memory.pyplugins/nemo-agents/tests/unit/test_runner_deployments.pyplugins/nemo-deployments/openapi/openapi.yamlplugins/nemo-deployments/src/nemo_deployments_plugin/auth_proxy.pyplugins/nemo-deployments/src/nemo_deployments_plugin/entities.pyplugins/nemo-deployments/tests/unit/test_auth_proxy.pyservices/core/inference-gateway/src/nmp/core/inference_gateway/api/authz.pyservices/core/inference-gateway/src/nmp/core/inference_gateway/api/v2/models.pyservices/core/inference-gateway/src/nmp/core/inference_gateway/api/v2/openai.pyservices/core/inference-gateway/src/nmp/core/inference_gateway/api/v2/providers.pyservices/core/inference-gateway/tests/integration/test_igw_with_auth.pyservices/core/inference-gateway/tests/unit/test_authz.py
Contributor
|
benmccown
enabled auto-merge
July 30, 2026 21:51
Agent deployments previously authenticated as the service:agents principal, which resolves to the ServiceSystem role (platform-wide wildcard access). Scope a deployment's runtime platform access to the identity that created it by delegating via on-behalf-of, instead of granting admin-level reach. - workload-proxy sidecar: add optional NMP_AUTH_PROXY_ON_BEHALF_OF; stamp X-NMP-Principal-On-Behalf-Of on forwarded requests, and strip the inbound OBO header so a co-located workload cannot spoof the delegated identity. - deployments plugin: add DeploymentConfig.auth_proxy_sidecar_on_behalf_of and wire it into the auth-proxy sidecar env (regenerated plugin OpenAPI spec). - agents runner: thread the deployment creator (created_by) through the container backend into the sidecar OBO env at deploy time. - inference-gateway: enforce delegated workspace access on the proxy path. For a delegated service principal, verify the on-behalf-of user holds the required inference permission in the target workspace (via the PDP evaluated as the delegated user). Non-delegated callers are unchanged, preserving the existing internal service bypass. Without this, the IGW route gate takes the service bypass and never narrows on OBO. The control plane (controller reconcile + DeploymentConfig/Deployment entity writes) intentionally remains service:agents. Adds unit + integration coverage for the sidecar OBO stamping/spoof-stripping, the DeploymentConfig wiring, the created_by threading, and IGW delegated allow/deny behavior. Signed-off-by: Ben McCown <bmccown@nvidia.com>
Address PR review feedback on the auth-proxy sidecar: - Strip inbound X-NMP-Principal-On-Behalf-Of-Email and -Groups companion headers, not just the OBO id. The platform derives the delegated user's effective groups/email from these headers and feeds them to the PDP, so a co-located workload could otherwise pair our stamped OBO id with attacker-chosen groups/email and be authorized as those, defeating the scoping. Extend the spoof-strip tests to cover both companion headers in the configured and unconfigured paths. - Do not log the delegated principal id in the sidecar startup line; log a boolean (delegated=<bool>) instead to keep creator identifiers out of logs. Signed-off-by: Ben McCown <bmccown@nvidia.com>
benmccown
force-pushed
the
fix-agent-rbac
branch
from
July 30, 2026 22:07
916cfef to
271b7df
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/core/inference-gateway/src/nmp/core/inference_gateway/api/authz.py`:
- Around line 78-90: Update the authorization-denial handling around the
principal permission check to stop exposing principal.on_behalf_of. Remove it
from the logger.info arguments and use only non-identifying context such as
permission, workspace, and service; replace the HTTPException detail with a
generic inference authorization denial that does not include delegated-principal
data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f6c1ff43-bb51-4463-88ed-2f15d9ce1857
📒 Files selected for processing (17)
packages/nmp_common/src/nmp/common/auth/workload_proxy/main.pypackages/nmp_common/tests/auth/test_workload_proxy.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/backend.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/controller.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/in_memory.pyplugins/nemo-agents/tests/unit/test_runner_deployments.pyplugins/nemo-deployments/openapi/openapi.yamlplugins/nemo-deployments/src/nemo_deployments_plugin/auth_proxy.pyplugins/nemo-deployments/src/nemo_deployments_plugin/entities.pyplugins/nemo-deployments/tests/unit/test_auth_proxy.pyservices/core/inference-gateway/src/nmp/core/inference_gateway/api/authz.pyservices/core/inference-gateway/src/nmp/core/inference_gateway/api/v2/models.pyservices/core/inference-gateway/src/nmp/core/inference_gateway/api/v2/openai.pyservices/core/inference-gateway/src/nmp/core/inference_gateway/api/v2/providers.pyservices/core/inference-gateway/tests/integration/test_igw_with_auth.pyservices/core/inference-gateway/tests/unit/test_authz.py
4 tasks
This was referenced Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Agent deployments previously authenticated as the
service:agentsprincipal, which resolves to theServiceSystemrole — a platform-wide wildcard (*) grant. That gave every deployed agent admin-level reach across all workspaces. This PR scopes a deployment's runtime platform access to the identity that created it, using the platform's existing on-behalf-of (OBO) delegation mechanism.The control plane (controller reconcile loop +
DeploymentConfig/Deploymententity writes) intentionally staysservice:agents— only the deployed agent's own platform calls are delegated.Motivation
service:agentswas hardcoded when auth support was added toAgentDeployment. Realistically a deployed agent should only reach the workspace(s) its creator can. The OBO stack already exists platform-wide (Principal.on_behalf_of/effective_principal,X-NMP-Principal-On-Behalf-Of, and the entity store already computes access for the OBO user), and the entitycreated_byis server-populated from the creating principal — so we delegate to that.PR Review Guide
The idea: a deployed agent was authenticating as
service:agents(platform-wide admin). Now it delegates to its creator via on-behalf-of, so it's scoped to the creator's workspaces. 17 files, but really 3 small moves + tests + 1 regenerated artifact.1. Sidecar learns "on behalf of (OBO)" — the core mechanism
packages/nmp_common/.../auth/workload_proxy/main.py— new optionalNMP_AUTH_PROXY_ON_BEHALF_OFenv; stampsX-NMP-Principal-On-Behalf-Ofon forwarded requests. Added the inbound OBO header to the strip-set so a workload can't spoof it. This is the heart of the change.2. Plumb the creator through to that env var
plugins/nemo-deployments/.../entities.py— newDeploymentConfig.auth_proxy_sidecar_on_behalf_offield.plugins/nemo-deployments/.../auth_proxy.py— passes that field into the sidecar'sNMP_AUTH_PROXY_ON_BEHALF_OFenv (appended only when set).plugins/nemo-agents/.../runner/deployments_backend.py— sets it from the deployment'screated_bywhen auth is on (warns if no creator).plugins/nemo-agents/.../runner/{backend,controller,in_memory}.py— threadcreated_bythrough thecreate_deploymentinterface; controller passesdep.created_by; subprocess backend ignores it (no sidecar).3. Make IGW actually honor "on behalf of"
services/core/inference-gateway/.../api/authz.py(new) —enforce_delegated_workspace_access(workspace, permission): for a delegated service principal, check the OBO user holds the requiredinference.gateway.*.execperm in the workspace (PDP evaluated as that user). No-op for everyone else, so no behavior change for normal callers.services/core/inference-gateway/.../api/v2/{openai,models,providers}.py— call that helper before routing. Needed because the shared route gate takes the service bypass and never narrows on OBO — without this, stamping OBO would do nothing for inference.Not changed on purpose
DeploymentConfig/Deploymentwrites) staysservice:agents.Mechanical / low-attention
plugins/nemo-deployments/openapi/openapi.yaml— regenerated (make refresh-openapi); just the one new field. Root SDK/CLI unaffected.test_workload_proxy.py(OBO stamp + spoof-strip),test_auth_proxy.py(env wiring),test_runner_deployments.py(created_by threading),test_authz.py(new) +test_igw_with_auth.py(delegated allow/deny).Suggested read order:
workload_proxy/main.py→inference-gateway/api/authz.py→ the deployments/agents plumbing → skim tests.Testing
DeploymentConfigenv wiring;created_bythreading (incl. no-creator fallback); new IGWtest_authz.py(no-op paths + delegated allow/deny).TestIGWDelegatedServicePrincipalAccess— delegated list-models and openai-proxy denied (403) when the OBO user lacks a role, allowed when granted. ExistingTestIGWServicePrincipalAccess(non-delegated bypass) still passes → no regression.ty,ruff, and pre-commit hooks clean.Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests