feat(auth): implement workload identity token exchange#631
Open
ironcommit wants to merge 9 commits into
Open
Conversation
ironcommit
changed the title
Contributor
|
🌿 Preview your docs: https://nvidia-preview-auth-idp-3-rsadler.docs.buildwithfern.com/nemo-platform |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
ironcommit
force-pushed
the
auth-idp-3/rsadler
branch
from
5175bba to
3795433
Compare
Contributor
|
Contributor
📝 WalkthroughWalkthroughThis PR adds RFC 8693 workload identity token exchange across platform auth, SDK clients, Docker and Kubernetes job backends, Authentik demos, API schemas, documentation, and CI coverage. ChangesWorkload identity exchange
Sequence Diagram(s)sequenceDiagram
participant Job
participant SDK
participant Gateway
participant Auth
Job->>SDK: Read subject token file
SDK->>Gateway: POST RFC 8693 token exchange
Gateway->>Auth: Validate subject token
Auth-->>Gateway: Signed platform access token
Gateway-->>SDK: Access token response
SDK->>Gateway: Authenticated API request
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions 🔧 Trivy (0.69.3)Trivy execution failed: 2026-07-14T00:48:52Z FATAL Fatal error run error: fs scan error: scan error: scan failed: failed analysis: post analysis error: post analysis error: cloudformation scan error: fs filter error: fs filter error: walk error range error: stat .coderabbit-oasdiff.yaml: no such file or directory: range error: stat .coderabbit-oasdiff.yaml: no such file or directory 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: 3
🧹 Nitpick comments (12)
contrib/auth/authentik/kubernetes/postgres.yaml (1)
27-42: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueAdd securityContext to demo Postgres deployment.
Checkov flags missing
allowPrivilegeEscalation: falseand non-root enforcement. Since this is a reference deployment others may copy, worth hardening even though demo-only.🔒 Suggested hardening
containers: - name: postgres image: docker.io/library/postgres:16-alpine + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true envFrom:🤖 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 `@contrib/auth/authentik/kubernetes/postgres.yaml` around lines 27 - 42, The postgres container in the demo deployment is missing basic hardening. Update the postgres container spec to include a securityContext that disables privilege escalation and enforces non-root execution, and keep the change scoped to the postgres workload so the reference deployment is safer to copy. Use the existing postgres container definition in the Kubernetes manifest to add the required security settings.Source: Linters/SAST tools
contrib/auth/authentik/kubernetes/workload-token-exchange.yaml (1)
45-73: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd container securityContext for broker.
Same as the gateway deployment: no
securityContext, root/privilege-escalation allowed (Checkov CKV_K8S_20/CKV_K8S_23).🔒 Suggested hardening
containers: - name: broker image: my-registry/nmp-api:local workingDir: /broker command: ["python", "-m", "uvicorn", "workload_exchange_broker:app", "--host", "0.0.0.0", "--port", "8080"] + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true env:🤖 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 `@contrib/auth/authentik/kubernetes/workload-token-exchange.yaml` around lines 45 - 73, The broker container spec is missing a securityContext, so it still runs with root/privilege-escalation defaults. Update the container definition in the workload-token-exchange manifest by adding the same hardening used by the gateway deployment, and make sure the broker container explicitly disables privilege escalation and runs with a non-root security context.Source: Linters/SAST tools
contrib/auth/authentik/kubernetes/envoy-gateway.yaml (1)
16-26: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd container securityContext.
No
securityContextset; container can run as root with privilege escalation allowed (Checkov CKV_K8S_20/CKV_K8S_23).🔒 Suggested hardening
containers: - name: envoy image: envoyproxy/envoy:v1.33-latest args: ["envoy", "-c", "/etc/envoy/envoy.yaml"] + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true ports:🤖 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 `@contrib/auth/authentik/kubernetes/envoy-gateway.yaml` around lines 16 - 26, The envoy container spec is missing a securityContext, leaving it able to run as root and allow privilege escalation. Update the container definition in the envoy gateway manifest by adding a container-level securityContext for the envoy container with non-root and no privilege escalation settings, and ensure the hardening is applied directly on the envoy container block.Source: Linters/SAST tools
packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py (1)
51-53: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUpdate docstring: direct mode no longer holds when workload token file env var is set.
Docstring says passing only
base_urlskips config bootstrap and auth header injection. Withhas_workload_identity_token_filenow included inshould_bootstrap, that's no longer true wheneverNMP_WORKLOAD_IDENTITY_TOKEN_FILEis set in the environment — bootstrap (and OIDC discovery) will run even for an explicitbase_url. Update the docs so callers relying on "direct mode" understand the exception.Also applies to: 78-81, 104-111, 199-201, 239-242, 265-272
🤖 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/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py` around lines 51 - 53, The docstrings for enhanced client direct mode are now outdated because `has_workload_identity_token_file` changes `should_bootstrap` behavior. Update the affected docstrings in `EnhancedClient`-related methods so they state that passing only `base_url` skips config bootstrap and auth injection only when `NMP_WORKLOAD_IDENTITY_TOKEN_FILE` is not set; otherwise bootstrap/OIDC discovery still runs. Keep the wording consistent across the referenced docstrings and mention the `should_bootstrap`/`has_workload_identity_token_file` exception explicitly.contrib/auth/authentik/kubernetes/redis.yaml (1)
16-28: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd securityContext to redis container.
No
securityContextset; container can run as root with privilege escalation allowed, per Checkov CKV_K8S_20/CKV_K8S_23.🔒 Proposed fix
containers: - name: redis image: docker.io/library/redis:7-alpine + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true + runAsUser: 999 args: ["--save", "60", "1", "--loglevel", "warning"]🤖 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 `@contrib/auth/authentik/kubernetes/redis.yaml` around lines 16 - 28, The redis container in the Kubernetes manifest is missing a securityContext, so it can run as root and allow privilege escalation. Add a container-level securityContext under the redis container definition and configure it to run as a non-root user with privilege escalation disabled, using the redis container block and its pod spec fields to place the change correctly. Ensure the settings satisfy the Checkov CKV_K8S_20 and CKV_K8S_23 requirements without changing the existing container behavior otherwise.Source: Linters/SAST tools
contrib/auth/authentik/kubernetes/authentik.yaml (1)
28-31: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueAdd a hardening
securityContextto both deployments.Demos get copied. Add
runAsNonRoot: trueandallowPrivilegeEscalation: falseso the reference doesn't ship a root/privesc-capable pod spec.Proposed securityContext
- name: authentik image: ghcr.io/goauthentik/server:2024.12 args: ["server"] + securityContext: + runAsNonRoot: true + allowPrivilegeEscalation: falseAlso applies to: 82-85
🤖 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 `@contrib/auth/authentik/kubernetes/authentik.yaml` around lines 28 - 31, Add a hardening securityContext to both Authentik deployment pod specs so they do not ship with a root- or privilege-escalation-capable container. Update the pod/container configuration around the authentik container definition to include runAsNonRoot: true and allowPrivilegeEscalation: false, and apply the same change to the second deployment mentioned in the review. Keep the change scoped to the deployment manifests so the existing image and args stay unchanged.Source: Linters/SAST tools
packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py (1)
98-99: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider excluding
tokensfrom repr, consistent with_lock.
tokens(containing the live access token) lacksrepr=Falsewhile_lockalready has it — suggests an oversight rather than a deliberate choice. Same class of exposure risk as flagged inworkload_tokens.py.🔒 Proposed fix
- tokens: TokenSet = field(default_factory=lambda: TokenSet(access_token="")) + tokens: TokenSet = field(default_factory=lambda: TokenSet(access_token=""), repr=False)🤖 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/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py` around lines 98 - 99, The `tokens` field in the workload exchange dataclass is still included in the generated repr, unlike `_lock`, which looks like an oversight and can expose live access token data. Update the dataclass definition in `workload_exchange.py` so `tokens` is declared with `repr=False`, matching the existing `_lock` handling and keeping the repr safe.services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py (1)
43-82: 🔒 Security & Privacy | 🔵 Trivial | ⚖️ Poor tradeoffROPC (password) grant used to obtain the subject token.
RFC 9700 explicitly states "Resource Owner Password Credentials Grant The resource owner password credentials grant [RFC6749] MUST NOT be used." Even labeled "demo," this issuer directly handles raw username/password and normalizes an insecure pattern. If this can be wired into any production/non-demo path, replace with client_credentials or a proper federated flow.
🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py` around lines 43 - 82, The OAuthPasswordGrantSubjectTokenIssuer.issue flow is using the insecure password grant and directly handling raw username/password credentials, which should not be used in production paths. Replace this issuer’s ROPC-based request construction and token exchange with a safer flow such as client_credentials or a federated token exchange, and update any callers of OAuthPasswordGrantSubjectTokenIssuer and SubjectToken issuance so they no longer depend on username/password inputs.services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py (1)
127-147: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSilent fallback hides workload-identity misconfig.
A transient/config error makes
is_workload_identity_token_exchange_enabled()returnFalse, so jobs launch without a subject token and fail auth downstream with no clear signal (debug log only). Log atwarningso operators can diagnose.Proposed change
except Exception: - logger.debug("Could not resolve auth config for workload identity token exchange", exc_info=True) + logger.warning("Could not resolve auth config for workload identity token exchange", exc_info=True) return Falseexcept Exception: - logger.debug("Could not resolve auth config for workload identity audience", exc_info=True) + logger.warning("Could not resolve auth config for workload identity audience", exc_info=True) return "nemo-platform"🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py` around lines 127 - 147, The silent fallback in is_workload_identity_token_exchange_enabled masks workload-identity misconfiguration by returning False on any config resolution error; update this helper to log at warning level instead of debug when get_auth_config() fails, keeping the False fallback but surfacing the problem clearly. Make the same logging-level adjustment in get_workload_identity_token_audience if it follows the same pattern, so both auth-config lookup paths in base.py are visible to operators when they fall back to defaults.services/core/jobs/tests/controllers/test_docker_backend.py (1)
707-747: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRefresher cleanup skipped if assertions fail.
refresher.stop()only runs if every assertion above it passes. Wrap infinallyso background refresher threads aren't leaked on assertion failure.♻️ Proposed fix
- job_create_call = next( - call - for call in docker_client_mock.containers.create.call_args_list - if call.kwargs.get("name") == "job-test-job-id-test-step" - ) - kwargs = job_create_call.kwargs - env = kwargs["environment"] - assert env[WORKLOAD_IDENTITY_TOKEN_FILE_ENVVAR] == WORKLOAD_IDENTITY_TOKEN_FILE_PATH - - mounts = kwargs["mounts"] - workload_identity_mount = next(m for m in mounts if m["Target"] == WORKLOAD_IDENTITY_VOLUME_PATH) - assert workload_identity_mount["Type"] == "volume" - assert workload_identity_mount["Source"].startswith( - f"task-workload-identity-{test_job_step.workspace}-{test_job_step.job}-" - ) - assert workload_identity_mount["ReadOnly"] is True - assert any( - call.kwargs.get("name", "").startswith("workload-token-write-") - for call in docker_client_mock.containers.create.call_args_list - ) - for refresher in list(docker_job._workload_identity_refreshers.values()): - refresher.stop() + try: + job_create_call = next( + call + for call in docker_client_mock.containers.create.call_args_list + if call.kwargs.get("name") == "job-test-job-id-test-step" + ) + kwargs = job_create_call.kwargs + env = kwargs["environment"] + assert env[WORKLOAD_IDENTITY_TOKEN_FILE_ENVVAR] == WORKLOAD_IDENTITY_TOKEN_FILE_PATH + + mounts = kwargs["mounts"] + workload_identity_mount = next(m for m in mounts if m["Target"] == WORKLOAD_IDENTITY_VOLUME_PATH) + assert workload_identity_mount["Type"] == "volume" + assert workload_identity_mount["Source"].startswith( + f"task-workload-identity-{test_job_step.workspace}-{test_job_step.job}-" + ) + assert workload_identity_mount["ReadOnly"] is True + assert any( + call.kwargs.get("name", "").startswith("workload-token-write-") + for call in docker_client_mock.containers.create.call_args_list + ) + finally: + for refresher in list(docker_job._workload_identity_refreshers.values()): + refresher.stop()🤖 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 `@services/core/jobs/tests/controllers/test_docker_backend.py` around lines 707 - 747, The test in test_docker_job_injects_workload_identity_volume_when_token_exchange_enabled leaks background refresher threads if any assertion fails before the cleanup loop. Update the test so the docker_job._workload_identity_refreshers cleanup runs from a finally block around the assertions and docker_client_mock checks, ensuring each refresher.stop() is always called even when an assertion aborts the test.contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py (2)
86-134: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSynchronous blocking network call inside an async handler.
_decode_kubernetes_subject_token(called from the asynctoken_exchangeroute) performs a blockingurllib.request.urlopenwith a 10s timeout directly on the event loop, stalling all other concurrent requests to the broker for up to 10s.Run it off the event loop, e.g.
await asyncio.to_thread(_decode_kubernetes_subject_token, subject_token, audience), or switch to an async HTTP client.Also applies to: 164-186
🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py` around lines 86 - 134, The blocking TokenReview call in _decode_kubernetes_subject_token is being executed from the async token_exchange flow, which can stall the event loop. Move the synchronous urllib.request.urlopen work off the loop by calling _decode_kubernetes_subject_token via asyncio.to_thread from token_exchange, or refactor the TokenReview request to use an async HTTP client. Make sure the async route no longer waits on a direct blocking network call while preserving the existing token validation and error handling in _decode_kubernetes_subject_token.
69-83: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSubject token
audis never validated.
options={"verify_aud": False}and noaudience=passed means the exchanged subject token's audience claim isn't checked, relying only on the issuer allowlist. For a reference implementation others may copy, verifyaudtoo.🔒 Proposed fix
claims = jwt.decode( subject_token, signing_key, algorithms=["RS256"], - options={"verify_aud": False}, + audience=AUDIENCE, leeway=30, )🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py` around lines 69 - 83, The subject token audience is not being checked in _decode_authentik_subject_token, so update the jwt.decode call to validate the token’s aud claim instead of disabling audience verification. Add an explicit expected audience value (or allowlist) to the decode path, keep the issuer check in place, and make sure the validation happens in the same auth flow used by _authentik_jwks_client so copied reference implementations enforce both iss and aud.
🤖 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/jobs/src/nmp/core/jobs/controllers/backends/docker.py`:
- Around line 1122-1124: The existing-container branch in DockerBackend should
not start the workload identity refresher early, because the same
SubjectTokenRefreshLoop is started again later and may be stopped immediately or
left running on early exits. Remove the
self._start_workload_identity_refresher(container.name,
workload_identity_refresher) call from the container-is-not-none path, and keep
the refresher startup centralized in the later docker backend flow so resumed
jobs still get token refresh correctly.
In
`@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py`:
- Around line 132-144: The _run() loop in workload_tokens.py retries
refresh_once() after a fixed _min_sleep_seconds on every exception, which can
hammer the token endpoint during persistent failures. Update _run() to use an
exponential or capped backoff after repeated refresh failures, resetting the
backoff after a successful refresh, and keep the existing logger.exception call
so the retry behavior in WorkloadToken refresh remains clear.
- Around line 42-53: The OAuthPasswordGrantSubjectTokenIssuer dataclass
currently exposes sensitive credentials in its default repr, unlike the _lock
handling elsewhere in the diff. Update the class definition so password and
client_secret are excluded from repr (similar to how other sensitive/internal
fields are handled), and keep the rest of the dataclass behavior unchanged. This
should be done in the OAuthPasswordGrantSubjectTokenIssuer symbol so accidental
logging or traceback inspection does not reveal secrets.
---
Nitpick comments:
In `@contrib/auth/authentik/kubernetes/authentik.yaml`:
- Around line 28-31: Add a hardening securityContext to both Authentik
deployment pod specs so they do not ship with a root- or
privilege-escalation-capable container. Update the pod/container configuration
around the authentik container definition to include runAsNonRoot: true and
allowPrivilegeEscalation: false, and apply the same change to the second
deployment mentioned in the review. Keep the change scoped to the deployment
manifests so the existing image and args stay unchanged.
In `@contrib/auth/authentik/kubernetes/envoy-gateway.yaml`:
- Around line 16-26: The envoy container spec is missing a securityContext,
leaving it able to run as root and allow privilege escalation. Update the
container definition in the envoy gateway manifest by adding a container-level
securityContext for the envoy container with non-root and no privilege
escalation settings, and ensure the hardening is applied directly on the envoy
container block.
In `@contrib/auth/authentik/kubernetes/postgres.yaml`:
- Around line 27-42: The postgres container in the demo deployment is missing
basic hardening. Update the postgres container spec to include a securityContext
that disables privilege escalation and enforces non-root execution, and keep the
change scoped to the postgres workload so the reference deployment is safer to
copy. Use the existing postgres container definition in the Kubernetes manifest
to add the required security settings.
In `@contrib/auth/authentik/kubernetes/redis.yaml`:
- Around line 16-28: The redis container in the Kubernetes manifest is missing a
securityContext, so it can run as root and allow privilege escalation. Add a
container-level securityContext under the redis container definition and
configure it to run as a non-root user with privilege escalation disabled, using
the redis container block and its pod spec fields to place the change correctly.
Ensure the settings satisfy the Checkov CKV_K8S_20 and CKV_K8S_23 requirements
without changing the existing container behavior otherwise.
In `@contrib/auth/authentik/kubernetes/workload-token-exchange.yaml`:
- Around line 45-73: The broker container spec is missing a securityContext, so
it still runs with root/privilege-escalation defaults. Update the container
definition in the workload-token-exchange manifest by adding the same hardening
used by the gateway deployment, and make sure the broker container explicitly
disables privilege escalation and runs with a non-root security context.
In `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Around line 86-134: The blocking TokenReview call in
_decode_kubernetes_subject_token is being executed from the async token_exchange
flow, which can stall the event loop. Move the synchronous
urllib.request.urlopen work off the loop by calling
_decode_kubernetes_subject_token via asyncio.to_thread from token_exchange, or
refactor the TokenReview request to use an async HTTP client. Make sure the
async route no longer waits on a direct blocking network call while preserving
the existing token validation and error handling in
_decode_kubernetes_subject_token.
- Around line 69-83: The subject token audience is not being checked in
_decode_authentik_subject_token, so update the jwt.decode call to validate the
token’s aud claim instead of disabling audience verification. Add an explicit
expected audience value (or allowlist) to the decode path, keep the issuer check
in place, and make sure the validation happens in the same auth flow used by
_authentik_jwks_client so copied reference implementations enforce both iss and
aud.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 98-99: The `tokens` field in the workload exchange dataclass is
still included in the generated repr, unlike `_lock`, which looks like an
oversight and can expose live access token data. Update the dataclass definition
in `workload_exchange.py` so `tokens` is declared with `repr=False`, matching
the existing `_lock` handling and keeping the repr safe.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py`:
- Around line 51-53: The docstrings for enhanced client direct mode are now
outdated because `has_workload_identity_token_file` changes `should_bootstrap`
behavior. Update the affected docstrings in `EnhancedClient`-related methods so
they state that passing only `base_url` skips config bootstrap and auth
injection only when `NMP_WORKLOAD_IDENTITY_TOKEN_FILE` is not set; otherwise
bootstrap/OIDC discovery still runs. Keep the wording consistent across the
referenced docstrings and mention the
`should_bootstrap`/`has_workload_identity_token_file` exception explicitly.
In `@services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py`:
- Around line 127-147: The silent fallback in
is_workload_identity_token_exchange_enabled masks workload-identity
misconfiguration by returning False on any config resolution error; update this
helper to log at warning level instead of debug when get_auth_config() fails,
keeping the False fallback but surfacing the problem clearly. Make the same
logging-level adjustment in get_workload_identity_token_audience if it follows
the same pattern, so both auth-config lookup paths in base.py are visible to
operators when they fall back to defaults.
In
`@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py`:
- Around line 43-82: The OAuthPasswordGrantSubjectTokenIssuer.issue flow is
using the insecure password grant and directly handling raw username/password
credentials, which should not be used in production paths. Replace this issuer’s
ROPC-based request construction and token exchange with a safer flow such as
client_credentials or a federated token exchange, and update any callers of
OAuthPasswordGrantSubjectTokenIssuer and SubjectToken issuance so they no longer
depend on username/password inputs.
In `@services/core/jobs/tests/controllers/test_docker_backend.py`:
- Around line 707-747: The test in
test_docker_job_injects_workload_identity_volume_when_token_exchange_enabled
leaks background refresher threads if any assertion fails before the cleanup
loop. Update the test so the docker_job._workload_identity_refreshers cleanup
runs from a finally block around the assertions and docker_client_mock checks,
ensuring each refresher.stop() is always called even when an assertion aborts
the test.
🪄 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: edca8b4b-086b-4ba6-b917-e737ef102df0
⛔ Files ignored due to path filters (9)
sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**
📒 Files selected for processing (62)
contrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/blueprints/nemo.yaml.tplcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tplcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kubernetes/workload-token-exchange.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.pycontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/tests/test_discovery.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.py
💤 Files with no reviewable changes (3)
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
contrib/auth/authentik/kubernetes/README.md (1)
82-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMissing "Next Steps" section.
File ends after the preflight section with no cross-links to related docs (e.g., Compose demo README, SDK workload identity docs).
As per coding guidelines: "Include 'Next Steps' section at the end with cross-links to related documentation content."
🤖 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 `@contrib/auth/authentik/kubernetes/README.md` around lines 82 - 101, The README currently ends after the Token Exchange Preflight section and is missing the required Next Steps cross-links. Add a “Next Steps” section at the end of the Authentik Kubernetes README and include links to related documentation such as the Compose demo README and the SDK workload identity docs, using the existing README structure and headings to place it after the preflight content.Source: Coding guidelines
packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
from __future__ import annotationsmakes all hints string-based.All imports here are already unconditional (no
TYPE_CHECKINGgating), so this import isn't strictly needed and turns concrete annotations into deferred strings, contrary to the guideline preference for concrete type hints.As per coding guidelines: "In Python code, prefer concrete type hints over string-based type hints, and do not import those types only under
TYPE_CHECKING; import them normally when possible."🤖 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/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py` at line 6, Remove the unnecessary future annotations import from workload_exchange.py so type hints remain concrete instead of string-based. Keep the existing imports used by the module as normal runtime imports, and ensure any annotations in the relevant functions or classes continue to reference the imported types directly rather than relying on deferred evaluation.Source: Coding guidelines
contrib/auth/authentik/kubernetes/postgres.yaml (1)
37-42: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPostgres data on
emptyDir— lost on any pod restart/reschedule.Authentik's entire state (users, blueprints, tokens) is wiped whenever this pod is rescheduled. Consider a
PersistentVolumeClaimso the demo survives restarts.Suggested fix
volumeMounts: - name: data mountPath: /var/lib/postgresql/data volumes: - - name: data - emptyDir: {} + - name: data + persistentVolumeClaim: + claimName: authentik-postgres-data🤖 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 `@contrib/auth/authentik/kubernetes/postgres.yaml` around lines 37 - 42, The Postgres storage in the Authentik Kubernetes manifest is using emptyDir in the postgres.yaml workload, which makes all database state disappear on pod restart or reschedule. Update the volume definition for the Postgres container to use a PersistentVolumeClaim instead of emptyDir, and ensure the existing volumeMounts for /var/lib/postgresql/data continue to point to that persistent volume. Keep the change scoped to the Postgres deployment/stateful resource so the database survives pod rescheduling.contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py (1)
46-46: 🩺 Stability & Availability | 🔵 TrivialSigning key regenerated on every restart; not scoped for >1 replica.
_private_keyis generated fresh at process start with no persistence. Any restart invalidates outstanding access tokens, and scaling beyondreplicas: 1would produce inconsistent JWKS across pods. Fine for the current single-replica demo; worth a comment/README note if this is ever scaled.🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py` at line 46, The signing key is currently generated at process startup in workload_exchange_broker.py via _private_key, which means restarts invalidate issued tokens and multiple replicas will expose different JWKS. Either persist and share the key across restarts/pods or, if this is intentionally demo-only, add a clear comment/README note near _private_key and the JWT/JWKS setup in this module stating it is only safe for a single-replica deployment and not for scaled use.
🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Around line 88-136: Both _decode_kubernetes_subject_token and
_decode_authentik_subject_token are doing blocking network I/O inside the async
token_exchange request path, which can stall the event loop. Move the
TokenReview and JWKS retrieval work off the event loop by making these fetches
asynchronous or running the blocking calls in a worker thread, and update
token_exchange to await the async helpers instead of calling them directly.
- Around line 37-44: The issuer allowlist currently fails open when
ALLOWED_SUBJECT_ISSUERS is empty, so the issuer validation in the workload
exchange path must not be skipped in that case. Update the issuer check logic
around ALLOWED_SUBJECT_ISSUERS so an empty set is treated as “reject all” (or
otherwise explicitly handled as invalid), and make the same fix anywhere the
issuer comparison is repeated in the workload exchange broker flow. Use the
ALLOWED_SUBJECT_ISSUERS symbol as the central source of truth and ensure the
condition in the issuer validation path always enforces the allowlist.
- Around line 190-199: The workload exchange flow in workload_exchange_broker.py
can raise an uncaught KeyError when building exchanged_claims because
subject_claims["sub"] is accessed after token decoding without validation.
Update the logic around the token exchange path to verify that subject_claims
contains a non-empty sub before constructing exchanged_claims, and if it is
missing, return the same sanitized invalid_grant error path used for other token
validation failures. Keep the fix localized near the existing subject_claims,
exchanged_claims, and invalid_grant handling so the exception never escapes the
try/except flow.
In `@openapi/openapi.yaml`:
- Around line 12575-12585: The workload identity token expiration fields
currently allow invalid values below Kubernetes’ minimum, so update both schema
entries for workload_identity_token_expiration_seconds in the OpenAPI definition
to enforce a minimum of 600. Make this change wherever the duplicated field
definition appears so API validation rejects unsupported
projected-service-account token expirations before job submission.
In
`@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py`:
- Around line 116-126: The start/stop lifecycle in the workload token refresher
is broken because stop() leaves _thread set, so start()’s guard in the same
class silently prevents restarting after a stop. Update the thread management in
start() and stop() for the workload token refresh loop so a stopped thread
clears _thread (and any needed stop state) after join, and make the start()
check use thread liveness rather than only whether _thread is non-None.
In
`@services/hello-world/tests/integration/tasks/test_workload_workspace_get_task.py`:
- Around line 33-40: The test is mocking out NeMoPlatform while also setting
NMP_WORKLOAD_IDENTITY_TOKEN_FILE, so the workload-token discovery and exchange
path is never actually exercised. Update test_workload_workspace_get_task to
either use a real temporary token file and let
nmp.hello_world.tasks.workload_workspace_get.run.NeMoPlatform run normally, or
remove the env var from this test and add a separate integration test for the
workload-token flow. Keep the assertions on task_run and
sdk.workspaces.requested, but make sure the test validates the real
discovery/provider behavior instead of a mocked constructor.
---
Nitpick comments:
In `@contrib/auth/authentik/kubernetes/postgres.yaml`:
- Around line 37-42: The Postgres storage in the Authentik Kubernetes manifest
is using emptyDir in the postgres.yaml workload, which makes all database state
disappear on pod restart or reschedule. Update the volume definition for the
Postgres container to use a PersistentVolumeClaim instead of emptyDir, and
ensure the existing volumeMounts for /var/lib/postgresql/data continue to point
to that persistent volume. Keep the change scoped to the Postgres
deployment/stateful resource so the database survives pod rescheduling.
In `@contrib/auth/authentik/kubernetes/README.md`:
- Around line 82-101: The README currently ends after the Token Exchange
Preflight section and is missing the required Next Steps cross-links. Add a
“Next Steps” section at the end of the Authentik Kubernetes README and include
links to related documentation such as the Compose demo README and the SDK
workload identity docs, using the existing README structure and headings to
place it after the preflight content.
In `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Line 46: The signing key is currently generated at process startup in
workload_exchange_broker.py via _private_key, which means restarts invalidate
issued tokens and multiple replicas will expose different JWKS. Either persist
and share the key across restarts/pods or, if this is intentionally demo-only,
add a clear comment/README note near _private_key and the JWT/JWKS setup in this
module stating it is only safe for a single-replica deployment and not for
scaled use.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Line 6: Remove the unnecessary future annotations import from
workload_exchange.py so type hints remain concrete instead of string-based. Keep
the existing imports used by the module as normal runtime imports, and ensure
any annotations in the relevant functions or classes continue to reference the
imported types directly rather than relying on deferred evaluation.
🪄 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: 7f7bb167-2386-41ce-8798-345e31ddb507
⛔ Files ignored due to path filters (13)
sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (66)
contrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/blueprints/nemo.yaml.tplcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tplcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kubernetes/workload-token-exchange.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.pycontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/tests/test_discovery.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.py
💤 Files with no reviewable changes (3)
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
✅ Files skipped from review due to trivial changes (6)
- tests/auth_idp/test_docs_links.py
- contrib/auth/authentik/.gitignore
- docs/auth/deployment/configuration.mdx
- contrib/auth/authentik/kubernetes/namespace.yaml
- docs/set-up/config-reference.mdx
- docs/auth/authentication/idp-integration.mdx
🚧 Files skipped from review as they are similar to previous changes (36)
- tests/auth_idp/test_fixture_helpers.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- tests/auth_idp/test_authentik_blueprint.py
- contrib/auth/manifest.schema.yaml
- packages/nmp_common/src/nmp/common/config/base.py
- contrib/auth/authentik/kustomization.yaml
- services/core/auth/tests/test_discovery.py
- contrib/auth/authentik/gateway/envoy.yaml
- tests/auth_idp/test_provider_manifest.py
- contrib/auth/authentik/manifest.yaml
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tpl
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- contrib/auth/authentik/blueprints/nemo.yaml
- packages/nemo_platform_ext/tests/config/test_config.py
- docs/auth/deployment/credential-propagation.mdx
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- contrib/auth/authentik/docker-compose.yml
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- contrib/auth/authentik/README.md
- contrib/auth/authentik/blueprints/nemo.yaml.tpl
- services/core/jobs/tests/controllers/test_workload_tokens.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- packages/nemo_platform_plugin/tests/test_client_auth.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
- contrib/auth/authentik/run.sh
- services/core/jobs/jobs-launcher/cmd/run.go
ironcommit
force-pushed
the
auth-idp-3/rsadler
branch
from
3795433 to
1abaee3
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py (2)
955-971: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClean up task volumes when initial token issuance fails.
If refresher construction or
refresh_once()raises, scheduling exits after creating all task volumes. No container exists for cleanup discovery, so each failed attempt leaks volumes.Proposed cleanup
- workload_identity_refresher = None - if workload_identity_volume_name is not None: - workload_identity_refresher = self._build_workload_identity_refresher(workload_identity_volume_name) - workload_identity_refresher.refresh_once() + try: + workload_identity_refresher = None + if workload_identity_volume_name is not None: + workload_identity_refresher = self._build_workload_identity_refresher(workload_identity_volume_name) + workload_identity_refresher.refresh_once() + except Exception: + self.cleanup_task_storage_volumes(step.workspace, step.job, task_id) + raise🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` around lines 955 - 971, Ensure task volumes are cleaned up when workload identity refresher construction or its initial refresh fails. In the code following ensure_job_storage, wrap _build_workload_identity_refresher and refresh_once in exception handling that removes the newly created task volumes through the existing cleanup mechanism before re-raising the scheduling error; preserve the normal refresher assignment and successful path.
947-971: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReuse the existing container’s workload-identity volume on resume.
Every scheduling attempt generates a new task ID and refresher before checking for an existing container. The resumed container remains mounted to its original volume, while refreshes target an unattached new volume. Resolve the existing container and its task label before provisioning token storage.
Also applies to: 1161-1161
🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` around lines 947 - 971, Resolve any existing container and read its original task label before generating or provisioning workload-identity storage. Update the scheduling/resume logic around ensure_job_storage and _build_workload_identity_refresher to reuse the existing container’s workload-identity volume and task identity; only create a new task ID, volume, and refresher when no container exists.
🤖 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 `@contrib/auth/authentik/kubernetes/authentik.yaml`:
- Around line 13-117: Add container-level securityContext hardening to both the
authentik-server and authentik-worker containers: set allowPrivilegeEscalation
to false and runAsNonRoot to true, preserving the existing container
configuration.
In `@contrib/auth/authentik/kubernetes/envoy-gateway.yaml`:
- Around line 16-30: Add a container-level securityContext to the envoy
container in the Deployment, setting allowPrivilegeEscalation to false and
runAsNonRoot to true, matching the hardening used in authentik.yaml.
In `@contrib/auth/authentik/kubernetes/workload-token-exchange.yaml`:
- Around line 43-49: Add a securityContext to the broker container in the
workload-token-exchange pod: run as the image’s non-root user, set
allowPrivilegeEscalation to false, drop all Linux capabilities, and configure
seccompProfile.type as RuntimeDefault. Place these settings alongside the
container fields identified by name broker.
In `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Around line 183-195: Validate the requested audience against an explicit
allowlist before calling _decode_subject_token or signing the exchanged token.
In the audience handling within the token exchange flow, reject unsupported
values with the existing _oauth_error response (for example, invalid_target or
invalid_request) and only copy an allowlisted audience into
exchanged_claims["aud"].
- Around line 75-84: Validate the subject token’s audience in the JWT decode
path instead of disabling audience verification with options={"verify_aud":
False}. In the Authentik exchange flow around jwt.decode and the issuer
allowlist check, require the configured expected audience (or explicitly compare
the decoded aud claim against it) and reject mismatches before exchanging the
token.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 46-70: Validate token_endpoint in token_exchange_grant before
calling httpx.post(): require HTTPS, allowing HTTP only for loopback hosts, and
reject all other non-HTTPS endpoints before transmitting subject_token. Reuse
the same endpoint-validation policy and symbols as the plugin implementation
where available.
In `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py`:
- Around line 321-337: The token-writing command in
_write_workload_identity_subject_token currently applies mode 0400, leaving the
root-owned token unreadable to non-root workloads. Update the command to chown
the token to the configured workload UID/GID, or apply an appropriate readable
mode for this dedicated volume, while preserving secure permissions.
- Line 258: The controller currently initializes _workload_identity_refreshers
empty, so active containers lose token refreshes after restart. During
controller startup, restore a SubjectTokenRefreshLoop for every existing ACTIVE
container and register it in _workload_identity_refreshers, reusing the same
scheduling/setup logic used for newly scheduled containers.
- Around line 404-427: Update _is_container_owned_by_this_controller, kill/stop
handling, and cleanup filtering to support legacy containers missing
JOB_CONTROLLER_INSTANCE_ID_LABEL. Preserve the strict owner-label checks for
newly created containers, but add a safe adoption path for matching legacy
containers identified by the existing controller/backend/profile criteria,
allowing them to be stopped, killed, and cleaned up during upgrades.
---
Outside diff comments:
In `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py`:
- Around line 955-971: Ensure task volumes are cleaned up when workload identity
refresher construction or its initial refresh fails. In the code following
ensure_job_storage, wrap _build_workload_identity_refresher and refresh_once in
exception handling that removes the newly created task volumes through the
existing cleanup mechanism before re-raising the scheduling error; preserve the
normal refresher assignment and successful path.
- Around line 947-971: Resolve any existing container and read its original task
label before generating or provisioning workload-identity storage. Update the
scheduling/resume logic around ensure_job_storage and
_build_workload_identity_refresher to reuse the existing container’s
workload-identity volume and task identity; only create a new task ID, volume,
and refresher when no container exists.
🪄 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: 91cfadde-884e-4c7b-982b-a56e5aa08a18
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (68)
contrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/blueprints/nemo.yaml.tplcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tplcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kubernetes/workload-token-exchange.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.pycontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/tests/test_discovery.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.py
💤 Files with no reviewable changes (3)
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
✅ Files skipped from review due to trivial changes (6)
- contrib/auth/authentik/kustomization.yaml
- docs/auth/authentication/idp-integration.mdx
- contrib/auth/authentik/kubernetes/namespace.yaml
- contrib/auth/authentik/.gitignore
- docs/set-up/config-reference.mdx
- docs/auth/deployment/configuration.mdx
🚧 Files skipped from review as they are similar to previous changes (40)
- tests/auth_idp/test_docs_links.py
- tests/auth_idp/test_fixture_helpers.py
- contrib/auth/manifest.schema.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- contrib/auth/authentik/gateway/envoy.yaml
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- packages/nmp_common/src/nmp/common/config/base.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- tests/auth_idp/test_provider_manifest.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- contrib/auth/authentik/docker-compose.yml
- services/core/auth/tests/test_discovery.py
- tests/auth_idp/test_authentik_blueprint.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tpl
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- contrib/auth/authentik/blueprints/nemo.yaml
- services/core/jobs/jobs-launcher/cmd/run.go
- packages/nemo_platform_plugin/tests/test_client_auth.py
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- contrib/auth/authentik/manifest.yaml
- services/hello-world/tests/integration/tasks/test_workload_workspace_get_task.py
- docs/auth/deployment/credential-propagation.mdx
- services/core/jobs/tests/controllers/test_workload_tokens.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- openapi/ga/individual/platform.openapi.yaml
- packages/nemo_platform_ext/tests/config/test_config.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- contrib/auth/authentik/blueprints/nemo.yaml.tpl
- contrib/auth/authentik/README.md
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- openapi/openapi.yaml
- contrib/auth/authentik/run.sh
- openapi/ga/openapi.yaml
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py (2)
955-971: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClean up task volumes when initial token issuance fails.
If refresher construction or
refresh_once()raises, scheduling exits after creating all task volumes. No container exists for cleanup discovery, so each failed attempt leaks volumes.Proposed cleanup
- workload_identity_refresher = None - if workload_identity_volume_name is not None: - workload_identity_refresher = self._build_workload_identity_refresher(workload_identity_volume_name) - workload_identity_refresher.refresh_once() + try: + workload_identity_refresher = None + if workload_identity_volume_name is not None: + workload_identity_refresher = self._build_workload_identity_refresher(workload_identity_volume_name) + workload_identity_refresher.refresh_once() + except Exception: + self.cleanup_task_storage_volumes(step.workspace, step.job, task_id) + raise🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` around lines 955 - 971, Ensure task volumes are cleaned up when workload identity refresher construction or its initial refresh fails. In the code following ensure_job_storage, wrap _build_workload_identity_refresher and refresh_once in exception handling that removes the newly created task volumes through the existing cleanup mechanism before re-raising the scheduling error; preserve the normal refresher assignment and successful path.
947-971: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReuse the existing container’s workload-identity volume on resume.
Every scheduling attempt generates a new task ID and refresher before checking for an existing container. The resumed container remains mounted to its original volume, while refreshes target an unattached new volume. Resolve the existing container and its task label before provisioning token storage.
Also applies to: 1161-1161
🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` around lines 947 - 971, Resolve any existing container and read its original task label before generating or provisioning workload-identity storage. Update the scheduling/resume logic around ensure_job_storage and _build_workload_identity_refresher to reuse the existing container’s workload-identity volume and task identity; only create a new task ID, volume, and refresher when no container exists.
🤖 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 `@contrib/auth/authentik/kubernetes/authentik.yaml`:
- Around line 13-117: Add container-level securityContext hardening to both the
authentik-server and authentik-worker containers: set allowPrivilegeEscalation
to false and runAsNonRoot to true, preserving the existing container
configuration.
In `@contrib/auth/authentik/kubernetes/envoy-gateway.yaml`:
- Around line 16-30: Add a container-level securityContext to the envoy
container in the Deployment, setting allowPrivilegeEscalation to false and
runAsNonRoot to true, matching the hardening used in authentik.yaml.
In `@contrib/auth/authentik/kubernetes/workload-token-exchange.yaml`:
- Around line 43-49: Add a securityContext to the broker container in the
workload-token-exchange pod: run as the image’s non-root user, set
allowPrivilegeEscalation to false, drop all Linux capabilities, and configure
seccompProfile.type as RuntimeDefault. Place these settings alongside the
container fields identified by name broker.
In `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Around line 183-195: Validate the requested audience against an explicit
allowlist before calling _decode_subject_token or signing the exchanged token.
In the audience handling within the token exchange flow, reject unsupported
values with the existing _oauth_error response (for example, invalid_target or
invalid_request) and only copy an allowlisted audience into
exchanged_claims["aud"].
- Around line 75-84: Validate the subject token’s audience in the JWT decode
path instead of disabling audience verification with options={"verify_aud":
False}. In the Authentik exchange flow around jwt.decode and the issuer
allowlist check, require the configured expected audience (or explicitly compare
the decoded aud claim against it) and reject mismatches before exchanging the
token.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 46-70: Validate token_endpoint in token_exchange_grant before
calling httpx.post(): require HTTPS, allowing HTTP only for loopback hosts, and
reject all other non-HTTPS endpoints before transmitting subject_token. Reuse
the same endpoint-validation policy and symbols as the plugin implementation
where available.
In `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py`:
- Around line 321-337: The token-writing command in
_write_workload_identity_subject_token currently applies mode 0400, leaving the
root-owned token unreadable to non-root workloads. Update the command to chown
the token to the configured workload UID/GID, or apply an appropriate readable
mode for this dedicated volume, while preserving secure permissions.
- Line 258: The controller currently initializes _workload_identity_refreshers
empty, so active containers lose token refreshes after restart. During
controller startup, restore a SubjectTokenRefreshLoop for every existing ACTIVE
container and register it in _workload_identity_refreshers, reusing the same
scheduling/setup logic used for newly scheduled containers.
- Around line 404-427: Update _is_container_owned_by_this_controller, kill/stop
handling, and cleanup filtering to support legacy containers missing
JOB_CONTROLLER_INSTANCE_ID_LABEL. Preserve the strict owner-label checks for
newly created containers, but add a safe adoption path for matching legacy
containers identified by the existing controller/backend/profile criteria,
allowing them to be stopped, killed, and cleaned up during upgrades.
---
Outside diff comments:
In `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py`:
- Around line 955-971: Ensure task volumes are cleaned up when workload identity
refresher construction or its initial refresh fails. In the code following
ensure_job_storage, wrap _build_workload_identity_refresher and refresh_once in
exception handling that removes the newly created task volumes through the
existing cleanup mechanism before re-raising the scheduling error; preserve the
normal refresher assignment and successful path.
- Around line 947-971: Resolve any existing container and read its original task
label before generating or provisioning workload-identity storage. Update the
scheduling/resume logic around ensure_job_storage and
_build_workload_identity_refresher to reuse the existing container’s
workload-identity volume and task identity; only create a new task ID, volume,
and refresher when no container exists.
🪄 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: 91cfadde-884e-4c7b-982b-a56e5aa08a18
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (68)
contrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/blueprints/nemo.yaml.tplcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tplcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kubernetes/workload-token-exchange.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.pycontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/tests/test_discovery.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.py
💤 Files with no reviewable changes (3)
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
✅ Files skipped from review due to trivial changes (6)
- contrib/auth/authentik/kustomization.yaml
- docs/auth/authentication/idp-integration.mdx
- contrib/auth/authentik/kubernetes/namespace.yaml
- contrib/auth/authentik/.gitignore
- docs/set-up/config-reference.mdx
- docs/auth/deployment/configuration.mdx
🚧 Files skipped from review as they are similar to previous changes (40)
- tests/auth_idp/test_docs_links.py
- tests/auth_idp/test_fixture_helpers.py
- contrib/auth/manifest.schema.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- contrib/auth/authentik/gateway/envoy.yaml
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- packages/nmp_common/src/nmp/common/config/base.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- tests/auth_idp/test_provider_manifest.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- contrib/auth/authentik/docker-compose.yml
- services/core/auth/tests/test_discovery.py
- tests/auth_idp/test_authentik_blueprint.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tpl
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- contrib/auth/authentik/blueprints/nemo.yaml
- services/core/jobs/jobs-launcher/cmd/run.go
- packages/nemo_platform_plugin/tests/test_client_auth.py
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- contrib/auth/authentik/manifest.yaml
- services/hello-world/tests/integration/tasks/test_workload_workspace_get_task.py
- docs/auth/deployment/credential-propagation.mdx
- services/core/jobs/tests/controllers/test_workload_tokens.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- openapi/ga/individual/platform.openapi.yaml
- packages/nemo_platform_ext/tests/config/test_config.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- contrib/auth/authentik/blueprints/nemo.yaml.tpl
- contrib/auth/authentik/README.md
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- openapi/openapi.yaml
- contrib/auth/authentik/run.sh
- openapi/ga/openapi.yaml
🛑 Comments failed to post (9)
contrib/auth/authentik/kubernetes/authentik.yaml (1)
13-117: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Add container
securityContexthardening.Neither
authentik-servernorauthentik-workersetsallowPrivilegeEscalation: falseorrunAsNonRoot: true. Since this is a reference others copy into real deployments, bake in the hardening now.🔒 Proposed fix
- name: authentik image: ghcr.io/goauthentik/server:2024.12 args: ["server"] + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true + capabilities: + drop: ["ALL"] env:🧰 Tools
🪛 Checkov (3.3.2)
[medium] 13-66: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[medium] 13-66: Minimize the admission of root containers
(CKV_K8S_23)
[medium] 67-117: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[medium] 67-117: Minimize the admission of root containers
(CKV_K8S_23)
🤖 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 `@contrib/auth/authentik/kubernetes/authentik.yaml` around lines 13 - 117, Add container-level securityContext hardening to both the authentik-server and authentik-worker containers: set allowPrivilegeEscalation to false and runAsNonRoot to true, preserving the existing container configuration.Source: Linters/SAST tools
contrib/auth/authentik/kubernetes/envoy-gateway.yaml (1)
16-30: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Add container
securityContexthardening.Same gap as
authentik.yaml: noallowPrivilegeEscalation: false/runAsNonRoot: trueon the envoy container.🔒 Proposed fix
- name: envoy image: envoyproxy/envoy:v1.33-latest args: ["envoy", "-c", "/etc/envoy/envoy.yaml"] + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true + capabilities: + drop: ["ALL"] ports:📝 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.containers: - name: envoy image: envoyproxy/envoy:v1.33-latest args: ["envoy", "-c", "/etc/envoy/envoy.yaml"] securityContext: allowPrivilegeEscalation: false runAsNonRoot: true capabilities: drop: ["ALL"] ports: - name: http containerPort: 8080 volumeMounts: - name: config mountPath: /etc/envoy readOnly: true volumes: - name: config configMap: name: authentik-envoy-config🤖 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 `@contrib/auth/authentik/kubernetes/envoy-gateway.yaml` around lines 16 - 30, Add a container-level securityContext to the envoy container in the Deployment, setting allowPrivilegeEscalation to false and runAsNonRoot to true, matching the hardening used in authentik.yaml.Source: Linters/SAST tools
contrib/auth/authentik/kubernetes/workload-token-exchange.yaml (1)
43-49: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== file listing ==" git ls-files contrib/auth/authentik/kubernetes echo echo "== outline ==" ast-grep outline contrib/auth/authentik/kubernetes/workload-token-exchange.yaml --view expanded || true echo echo "== relevant file with line numbers ==" cat -n contrib/auth/authentik/kubernetes/workload-token-exchange.yaml | sed -n '1,220p' echo echo "== search for securityContext in same area ==" rg -n "securityContext|runAsNonRoot|allowPrivilegeEscalation|capabilities|seccompProfile" contrib/auth/authentik/kubernetes -S || trueRepository: NVIDIA-NeMo/nemo-platform
Length of output: 3998
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== search image/broker references ==" rg -n "nmp-api:local|workload_exchange_broker|workload-token-exchange-broker|securityContext" -S . echo echo "== relevant README ==" cat -n contrib/auth/authentik/kubernetes/README.md | sed -n '1,220p' echo echo "== other k8s manifests with container securityContext ==" rg -n "securityContext:" contrib/auth/authentik/kubernetes -n -SRepository: NVIDIA-NeMo/nemo-platform
Length of output: 17423
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== auth run/docker files ==" git ls-files contrib/auth/authentik | sed -n '1,200p' echo echo "== docker-compose broker service ==" cat -n contrib/auth/authentik/docker-compose.yml | sed -n '1,180p' echo echo "== image build context references ==" rg -n "FROM |USER |nmp-api|workload-exchange-broker|workload-token-exchange" contrib/auth/authentik -SRepository: NVIDIA-NeMo/nemo-platform
Length of output: 13037
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== nmp-api image build files ==" rg -n "nmp-api|FROM .*python|USER " -S . --glob 'Dockerfile*' --glob '*.dockerfile' --glob 'compose*.yml' --glob '*.yaml' --glob '*.yml' | sed -n '1,240p' echo echo "== likely Dockerfile candidates ==" fd -a -t f 'Dockerfile*|*.dockerfile' . || trueRepository: NVIDIA-NeMo/nemo-platform
Length of output: 12145
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== docker/Dockerfile.nmp-api (surrounding lines) ==" cat -n docker/Dockerfile.nmp-api | sed -n '1,180p' echo echo "== workload exchange broker ==" cat -n contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py | sed -n '1,240p'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 14318
Add a broker securityContext. Run as the image’s non-root user and disable privilege escalation; drop all capabilities and set
seccompProfile: RuntimeDefault. This pod handles subject tokens and has TokenReview access.🤖 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 `@contrib/auth/authentik/kubernetes/workload-token-exchange.yaml` around lines 43 - 49, Add a securityContext to the broker container in the workload-token-exchange pod: run as the image’s non-root user, set allowPrivilegeEscalation to false, drop all Linux capabilities, and configure seccompProfile.type as RuntimeDefault. Place these settings alongside the container fields identified by name broker.Source: Linters/SAST tools
contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py (2)
75-84: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate the file and inspect the relevant section with line numbers. sed -n '1,220p' contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py | cat -n # Find any subject-token audience handling or related validation in this broker. rg -n "aud|audience|issuer|subject_token|verify_aud|InvalidAudienceError" contrib/auth/authentik/workload-exchange-broker -S # Inspect nearby auth/config code for any configured expected audience. rg -n "ALLOWED_SUBJECT_ISSUERS|allowed.*aud|subject.*aud|expected.*aud|audience" contrib/auth/authentik -SRepository: NVIDIA-NeMo/nemo-platform
Length of output: 15885
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the Authentik-related manifest and docs for any subject-token audience requirement. sed -n '1,220p' contrib/auth/authentik/manifest.yaml | cat -n printf '\n---\n' sed -n '1,220p' contrib/auth/authentik/kubernetes/README.md | cat -n printf '\n---\n' sed -n '1,220p' contrib/auth/authentik/kubernetes/workload-token-exchange.yaml | cat -n printf '\n---\n' sed -n '1,200p' contrib/auth/authentik/gateway/envoy.yaml | cat -nRepository: NVIDIA-NeMo/nemo-platform
Length of output: 16730
🏁 Script executed:
#!/bin/bash set -euo pipefail # Search for any explicit subject-token audience requirement or exception. rg -n "subject token|subject_token|audience.*subject|validate.*aud|verify_aud|token exchange" contrib/auth/authentik -S # Read the main Authentik README if present. fd -a -g 'README.md' contrib/auth/authentikRepository: NVIDIA-NeMo/nemo-platform
Length of output: 5738
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,140p' contrib/auth/authentik/README.md | cat -n printf '\n---\n' sed -n '140,260p' contrib/auth/authentik/README.md | cat -n printf '\n---\n' sed -n '1,120p' contrib/auth/authentik/config/platform-compose-authentik.yaml | cat -nRepository: NVIDIA-NeMo/nemo-platform
Length of output: 12569
Validate the Authentik subject-token audience
contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py:75-84verify_aud=Falseaccepts any JWT from the allowed issuer. Add a subject-tokenaudcheck on the Authentik path so tokens minted for other clients/resources can’t be exchanged.🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py` around lines 75 - 84, Validate the subject token’s audience in the JWT decode path instead of disabling audience verification with options={"verify_aud": False}. In the Authentik exchange flow around jwt.decode and the issuer allowlist check, require the configured expected audience (or explicitly compare the decoded aud claim against it) and reject mismatches before exchanging the token.
183-195: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject unsupported target audiences.
The caller controls
audience, which is copied into a broker-signed token without an allowlist. Any valid subject token can therefore mint credentials for arbitrary relying services that trust this issuer.Proposed fix
try: audience = str(form.get("audience") or AUDIENCE) + if audience != AUDIENCE: + return _oauth_error(400, "invalid_target", "Requested audience is not allowed") subject_claims = _decode_subject_token(str(subject_token), audience)📝 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.try: audience = str(form.get("audience") or AUDIENCE) if audience != AUDIENCE: return _oauth_error(400, "invalid_target", "Requested audience is not allowed") subject_claims = _decode_subject_token(str(subject_token), audience) except Exception: logger.exception("Subject token validation failed") return _oauth_error(400, "invalid_grant", "Could not validate subject token") now = int(time.time()) scope = str(form.get("scope") or SCOPE) exchanged_claims: dict[str, Any] = { "iss": ISSUER, "sub": subject_claims["sub"], "aud": audience,🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py` around lines 183 - 195, Validate the requested audience against an explicit allowlist before calling _decode_subject_token or signing the exchanged token. In the audience handling within the token exchange flow, reject unsupported values with the existing _oauth_error response (for example, invalid_target or invalid_request) and only copy an allowlisted audience into exchanged_claims["aud"].packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py (1)
46-70: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate the token endpoint before sending the subject credential.
Unlike the plugin implementation, this path accepts non-loopback HTTP URLs and uploads the workload subject token in cleartext. Apply the same HTTPS-with-loopback-exception policy before
httpx.post().🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 69-69: Request-controlled URL passed to httpx; validate against an allowlist to prevent SSRF.
Context: httpx.post(token_endpoint, data=data, timeout=timeout)
Note: [CWE-918] Server-Side Request Forgery (SSRF).(avoid-ssrf)
🤖 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/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py` around lines 46 - 70, Validate token_endpoint in token_exchange_grant before calling httpx.post(): require HTTPS, allowing HTTP only for loopback hosts, and reject all other non-HTTPS endpoints before transmitting subject_token. Reuse the same endpoint-validation policy and symbols as the plugin implementation where available.Source: Linters/SAST tools
services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py (3)
258-258: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Restore refreshers for active containers after controller restart.
The mapping starts empty and only scheduling populates it. Existing
ACTIVEcontainers therefore stop receiving token updates after a controller restart and eventually lose authentication.🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` at line 258, The controller currently initializes _workload_identity_refreshers empty, so active containers lose token refreshes after restart. During controller startup, restore a SubjectTokenRefreshLoop for every existing ACTIVE container and register it in _workload_identity_refreshers, reusing the same scheduling/setup logic used for newly scheduled containers.
321-337: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the token readable by non-root workloads.
put_archive()creates a root-owned file, then the writer applies mode0400. Job containers running as a non-root UID cannot read the configured token path. Chown it to the workload UID/GID or use an appropriate readable mode for this dedicated volume.🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` around lines 321 - 337, The token-writing command in _write_workload_identity_subject_token currently applies mode 0400, leaving the root-owned token unreadable to non-root workloads. Update the command to chown the token to the configured workload UID/GID, or apply an appropriate readable mode for this dedicated volume, while preserving secure permissions.
404-427: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Provide an adoption path for containers without the new owner label.
Containers created before this change lack
JOB_CONTROLLER_INSTANCE_ID_LABEL. They are still found by name, but kill, stop, and cleanup now reject or exclude them, stranding jobs during upgrades.Also applies to: 1467-1479, 1545-1556, 1773-1776
🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` around lines 404 - 427, Update _is_container_owned_by_this_controller, kill/stop handling, and cleanup filtering to support legacy containers missing JOB_CONTROLLER_INSTANCE_ID_LABEL. Preserve the strict owner-label checks for newly created containers, but add a safe adoption path for matching legacy containers identified by the existing controller/backend/profile criteria, allowing them to be stopped, killed, and cleaned up during upgrades.
ironcommit
force-pushed
the
auth-idp-3/rsadler
branch
from
1abaee3 to
33db07e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
contrib/auth/authentik/kubernetes/README.md (1)
1-100: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSeparate the how-to from the explanation.
This page mixes deployment steps with token-exchange architecture, has no Diataxis classification, lacks tabbed Python SDK/CLI task examples, and has no
Next Steps. Keep this as a HOW-TO; move lines 50-66 to an explanation page and cross-link it.🤖 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 `@contrib/auth/authentik/kubernetes/README.md` around lines 1 - 100, Keep this README focused as a Diataxis HOW-TO: add an explicit HOW-TO classification, retain only actionable deployment and verification steps, and move the token-exchange architecture explanation currently in “Start NeMo With The Kubernetes Override” to a separate explanation page. Add a cross-link to that page, provide tabbed Python SDK and CLI examples for submitting the workload task, and append a “Next Steps” section with relevant follow-up links.Source: Coding guidelines
🤖 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 `@contrib/auth/authentik/kubernetes/authentik.yaml`:
- Around line 27-116: Harden both Authentik pod templates in the authentik
server and authentik-worker Deployments: set automountServiceAccountToken to
false, add pod/container security contexts enforcing non-root execution,
RuntimeDefault seccomp, allowPrivilegeEscalation false, and drop all
capabilities. Before enabling readOnlyRootFilesystem, verify Authentik can write
to required paths and add explicit writable mounts such as emptyDir volumes
where necessary; apply the same validated configuration to both containers.
In `@contrib/auth/authentik/kubernetes/envoy-gateway.yaml`:
- Around line 15-30: Update the Envoy container in the gateway Pod spec with a
restrictive securityContext: run as non-root, set allowPrivilegeEscalation to
false, drop all Linux capabilities, use RuntimeDefault seccomp, and enable a
read-only root filesystem where supported. Set automountServiceAccountToken to
false at the Pod spec level to disable the unused token mount.
- Around line 17-26: Replace the mutable envoyproxy/envoy:v1.33-latest image
reference in the Envoy container with a specific immutable image digest, and
update the matching Envoy image reference in docker-compose.yml to use the same
pinned digest.
In `@contrib/auth/authentik/kubernetes/postgres.yaml`:
- Around line 40-42: Replace the ephemeral emptyDir volume in the Authentik
PostgreSQL pod with a PersistentVolumeClaim-backed volume, referencing a defined
claim for durable database storage; alternatively, document and automate the
intentional reset behavior if persistence is not required.
- Around line 27-42: Harden the PostgreSQL Pod defined by the container named
“postgres” by adding a non-root security context, disabling privilege
escalation, dropping all Linux capabilities, and setting seccomp to
RuntimeDefault; also disable the unused ServiceAccount token mount at the Pod
spec level with automountServiceAccountToken: false.
In `@contrib/auth/authentik/kubernetes/redis.yaml`:
- Around line 15-28: Update the Redis container in the pod spec to run as
non-root with privilege escalation disabled, drop all Linux capabilities, and
use the RuntimeDefault seccomp profile; also disable automatic ServiceAccount
token mounting at the pod level. Apply these settings around the existing redis
container and pod spec without changing its data volume configuration.
In `@contrib/auth/authentik/kubernetes/workload-token-exchange.yaml`:
- Around line 43-77: Add a securityContext to the broker container in the
workload-token-exchange manifest: run as a non-root user, disable privilege
escalation, drop all Linux capabilities, and set seccompProfile.type to
RuntimeDefault. Enable readOnlyRootFilesystem and add an emptyDir volume plus
matching mount only for the broker’s required writable paths, preserving the
existing ServiceAccount token access and application behavior.
In `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Around line 219-234: Restrict the audience used by the token exchange flow
instead of accepting arbitrary form input. Update the audience handling in the
subject-token validation block and the exchanged_claims construction to accept
only AUDIENCE or values from an explicit configured allowlist; reject disallowed
audiences with the existing invalid_grant response before signing the token.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 72-83: Update the response handling in _exchange() to validate
both error and successful JSON payloads: require JSON objects and, for 200
responses, a non-empty access_token; otherwise raise WorkloadTokenExchangeError
instead of allowing AttributeError or KeyError. Return a concrete dict[str,
object] and adjust parsing/type annotations accordingly.
In
`@services/hello-world/tests/integration/tasks/test_workload_workspace_get_task.py`:
- Around line 64-81: Update the test to avoid replacing sdk.workspaces.retrieve,
since that bypasses just-in-time authentication. Stub the underlying HTTP
transport used by task_run so the real SDK request executes, return a successful
response, and assert the transport receives the Authorization header Bearer
exchanged-access-token; update exchange_requests expectations to reflect the
actual exchange.
---
Nitpick comments:
In `@contrib/auth/authentik/kubernetes/README.md`:
- Around line 1-100: Keep this README focused as a Diataxis HOW-TO: add an
explicit HOW-TO classification, retain only actionable deployment and
verification steps, and move the token-exchange architecture explanation
currently in “Start NeMo With The Kubernetes Override” to a separate explanation
page. Add a cross-link to that page, provide tabbed Python SDK and CLI examples
for submitting the workload task, and append a “Next Steps” section with
relevant follow-up links.
🪄 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: 9139e7ab-6700-42b6-a4dc-47676a1bc083
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (69)
contrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/blueprints/nemo.yaml.tplcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tplcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kubernetes/workload-token-exchange.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.pycontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/tests/test_discovery.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_workload_exchange_broker.pytests/auth_idp_k8s/test_authentik_kubernetes_live.py
💤 Files with no reviewable changes (3)
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
✅ Files skipped from review due to trivial changes (8)
- contrib/auth/authentik/kubernetes/namespace.yaml
- docs/auth/authentication/idp-integration.mdx
- contrib/auth/authentik/kustomization.yaml
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- docs/auth/deployment/configuration.mdx
- contrib/auth/authentik/.gitignore
- tests/auth_idp/test_fixture_helpers.py
- tests/auth_idp/test_provider_manifest.py
🚧 Files skipped from review as they are similar to previous changes (39)
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- contrib/auth/authentik/manifest.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tpl
- tests/auth_idp/test_docs_links.py
- services/core/auth/tests/test_discovery.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- docs/auth/deployment/credential-propagation.mdx
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- tests/auth_idp/test_authentik_blueprint.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- packages/nemo_platform_plugin/tests/test_client_auth.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- e2e/services_pool.py
- contrib/auth/authentik/gateway/envoy.yaml
- packages/nmp_common/src/nmp/common/config/base.py
- contrib/auth/manifest.schema.yaml
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- contrib/auth/authentik/blueprints/nemo.yaml
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- contrib/auth/authentik/blueprints/nemo.yaml.tpl
- contrib/auth/authentik/docker-compose.yml
- services/core/jobs/jobs-launcher/cmd/run.go
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- packages/nemo_platform_ext/tests/config/test_config.py
- contrib/auth/authentik/README.md
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- docs/set-up/config-reference.mdx
- contrib/auth/authentik/run.sh
- openapi/ga/individual/platform.openapi.yaml
- e2e/conftest.py
- openapi/ga/openapi.yaml
- openapi/openapi.yaml
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
ironcommit
force-pushed
the
auth-idp-3/rsadler
branch
from
33db07e to
8c72a38
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Around line 102-108: Require the subject token to contain an expiration claim
by adding PyJWT’s required-claims option to the jwt.decode call in the token
validation flow. Update the decode configuration associated with subject_token
and ensure "exp" is included among the required claims while preserving the
existing algorithm, audience, and leeway settings.
In `@tests/auth_idp/test_authentik_kubernetes_demo.py`:
- Around line 85-98: Update
test_kubernetes_platform_override_enables_workload_identity_exchange and its
referenced platform configuration to require a secure workload-token endpoint:
configure HTTPS/mTLS as appropriate and change the endpoint assertion from
http:// to https:// while preserving the existing workload identity checks.
🪄 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: f938695c-a892-4133-98ff-23f5cd1f13e7
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (70)
contrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/blueprints/nemo.yaml.tplcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tplcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kubernetes/workload-token-exchange.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.pycontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/tests/test_discovery.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/core/jobs/tests/test_config.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_workload_exchange_broker.pytests/auth_idp_k8s/test_authentik_kubernetes_live.py
💤 Files with no reviewable changes (3)
- tests/auth_idp/test_authentik_real_oidc.py
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
✅ Files skipped from review due to trivial changes (5)
- contrib/auth/authentik/kubernetes/namespace.yaml
- contrib/auth/authentik/.gitignore
- docs/auth/authentication/idp-integration.mdx
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- docs/auth/deployment/configuration.mdx
🚧 Files skipped from review as they are similar to previous changes (42)
- contrib/auth/authentik/kustomization.yaml
- contrib/auth/manifest.schema.yaml
- tests/auth_idp/test_docs_links.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- tests/auth_idp/test_fixture_helpers.py
- tests/auth_idp/test_provider_manifest.py
- docs/auth/deployment/credential-propagation.mdx
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- packages/nmp_common/src/nmp/common/config/base.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- e2e/services_pool.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- contrib/auth/authentik/gateway/envoy.yaml
- contrib/auth/authentik/docker-compose.yml
- contrib/auth/authentik/blueprints/nemo.yaml
- services/core/jobs/jobs-launcher/cmd/run.go
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- packages/nemo_platform_plugin/tests/test_client_auth.py
- services/core/auth/tests/test_discovery.py
- services/core/jobs/tests/controllers/test_workload_tokens.py
- contrib/auth/authentik/run.sh
- contrib/auth/authentik/manifest.yaml
- e2e/conftest.py
- openapi/ga/individual/platform.openapi.yaml
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- packages/nemo_platform_ext/tests/config/test_config.py
- contrib/auth/authentik/README.md
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tpl
- openapi/ga/openapi.yaml
- openapi/openapi.yaml
- contrib/auth/authentik/blueprints/nemo.yaml.tpl
- docs/set-up/config-reference.mdx
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
ironcommit
force-pushed
the
auth-idp-3/rsadler
branch
from
7761bb1 to
a2c4c1d
Compare
mckornfield
reviewed
Jul 10, 2026
mckornfield
left a comment
mckornfield
left a comment
Contributor
There was a problem hiding this comment.
fun pr lol
personally my beef is just the authentik bit using kustomize, I'd rather it just be in helm if it's all the same, since we already have a helm chart in the repo (understanding it's a demo object, but unless you have a strong reason it should be kustomize, idk why we wouldn't just use helm)
|
|
||
| This directory contains a raw Kubernetes + Kustomize version of the Authentik | ||
| reference example. It is intended for local development clusters and uses | ||
| demo-only secrets. |
Contributor
There was a problem hiding this comment.
guess that answers my above q's lol. Do think a helm chart would be more consistent with what's already in the repo, no?
Contributor
There was a problem hiding this comment.
This is a lot to maintain vs their documented helm install they maintain? https://docs.goauthentik.io/install-config/install/kubernetes/#install-authentik-helm-chart
Contributor
There was a problem hiding this comment.
| if not isinstance(status, dict): | ||
| raise jwt.InvalidTokenError("Kubernetes TokenReview response did not include a status object") | ||
| if not status.get("authenticated"): | ||
| raise jwt.InvalidTokenError(status.get("error") or "Kubernetes TokenReview rejected subject token") |
Contributor
There was a problem hiding this comment.
maybe a nit, should we gather all the issues into a list of errors and then raise one thing at the end?
| """ | ||
| # Backward compatibility: an explicit base_url means direct mode (no config bootstrap), | ||
| # unless config-specific overrides are provided. | ||
| has_workload_identity_token_file = bool(os.environ.get("NMP_WORKLOAD_IDENTITY_TOKEN_FILE")) |
Contributor
There was a problem hiding this comment.
so if this is set, and we have no http client, we should bootstrap?
ironcommit
force-pushed
the
auth-idp-3/rsadler
branch
2 times, most recently
from
f2fb467 to
ba745d8
Compare
| @@ -0,0 +1,91 @@ | |||
| # Authentik Kubernetes Reference Demo | |||
Contributor
There was a problem hiding this comment.
They don't provide one for us to use?
Contributor
Author
There was a problem hiding this comment.
updated to use authentik official helm chart.
| python-version: "3.11" | ||
| enable-cache: true | ||
| cache-dependency-glob: uv.lock | ||
| - name: Install kind |
Contributor
There was a problem hiding this comment.
There is already a composite action to setup a kind cluster in .github/actions. It also installs the nemo-platform, but it can be made more generic if needed. I'd rather not duplicate it all a lot.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
contrib/auth/authentik/kubernetes/redis.yaml (1)
27-37: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
readOnlyRootFilesystem: true.
/datais already a dedicated volume; root fs doesn't need to stay writable.🔒 Proposed fix
securityContext: runAsNonRoot: true runAsUser: 999 allowPrivilegeEscalation: false + readOnlyRootFilesystem: true capabilities: drop: ["ALL"] seccompProfile: type: RuntimeDefault🤖 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 `@contrib/auth/authentik/kubernetes/redis.yaml` around lines 27 - 37, Add readOnlyRootFilesystem: true to the Redis pod’s securityContext alongside runAsNonRoot and allowPrivilegeEscalation settings, while keeping the existing /data volumeMount writable for Redis data.contrib/auth/authentik/kubernetes/README.md (1)
1-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a "Next Steps" section.
Doc ends abruptly after the preflight script section with no cross-links to related docs (Compose demo, auth deployment/configuration docs).
As per coding guidelines: "Include 'Next Steps' section at the end with cross-links to related documentation content."
🤖 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 `@contrib/auth/authentik/kubernetes/README.md` around lines 1 - 92, Add a “Next Steps” section at the end of the README after the k8s-test instructions, linking to the related Authentik Compose demo and the authentication deployment/configuration documentation using the repository’s correct relative Markdown paths.Source: Coding guidelines
contrib/auth/authentik/run.sh (1)
247-247: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded kustomization path duplicates
k8s_render'soutput_dir.
run_k8s_testshardcodescontrib/auth/authentik/.generated/kubernetesasNMP_AUTHENTIK_K8S_KUSTOMIZATION, whilek8s_rendercomputes the same path from${COMPOSE_DIR}/.generated/kubernetes. Currently safe since--compose-diris rejected fork8s-test, but the literal duplicates a value derivable fromCOMPOSE_DIR/REPO_ROOT, risking drift if either changes.♻️ Suggested fix
run_in_repo \ env "IMAGE_REGISTRY=${IMAGE_REGISTRY}" "BAKE_TAG=${BAKE_TAG}" \ - "NMP_AUTHENTIK_K8S_E2E=1" "NMP_AUTHENTIK_K8S_KUSTOMIZATION=contrib/auth/authentik/.generated/kubernetes" \ + "NMP_AUTHENTIK_K8S_E2E=1" "NMP_AUTHENTIK_K8S_KUSTOMIZATION=${COMPOSE_DIR#${REPO_ROOT}/}/.generated/kubernetes" \ uv run --frozen pytest tests/auth_idp_k8s -v --run-e2eAlso applies to: 328-330
🤖 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 `@contrib/auth/authentik/run.sh` at line 247, Update run_k8s_tests and the related kustomization setup to derive NMP_AUTHENTIK_K8S_KUSTOMIZATION from the same ${COMPOSE_DIR}/.generated/kubernetes expression used by k8s_render, instead of hardcoding the repository path. Reuse the existing output_dir or a shared variable so both code paths remain consistent if COMPOSE_DIR changes.packages/nmp_common/src/nmp/common/config/base.py (1)
198-212: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winEnforce the documented dependency between
workload_subject_jwks_uriandworkload_subject_issuers.The description states issuers are "Required when workload_subject_jwks_uri is set," but nothing enforces this. If an operator sets the JWKS URI without issuers,
_allowed_subject_issuersinworkload_token_exchange.pyreturns an empty set and every JWT-based exchange silently fails withinvalid_grant—a confusing, hard-to-diagnose misconfiguration.♻️ Suggested validator
+ `@model_validator`(mode="after") + def _validate_workload_subject_config(self) -> "OIDCConfig": + if self.workload_subject_jwks_uri and not self.workload_subject_issuers: + raise ValueError( + "workload_subject_issuers is required when workload_subject_jwks_uri is set" + ) + return self🤖 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/config/base.py` around lines 198 - 212, Enforce the documented dependency between BaseConfig fields workload_subject_jwks_uri and workload_subject_issuers: add a model-level validator that rejects configurations where workload_subject_jwks_uri is set while workload_subject_issuers is empty, with a clear validation error. Preserve valid configurations where the URI is unset or issuers are provided, and ensure the validator matches the project’s existing Pydantic validation style.services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py (1)
98-127: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUncached JWKS fetch on every token exchange request.
_fetch_subject_jwkshitsworkload_subject_jwks_uriover HTTP on every call, and_select_subject_signing_keyreimplements PyJWT's ownPyJWKClient.get_signing_keys()filtering from scratch. Considerjwt.PyJWKClient(jwks_uri)(built-in caching, key refresh-on-miss, and this exact "sig"/kid filtering) instead of hand-rolling fetch + filter logic per request.♻️ Simplification using PyJWKClient
-async def _fetch_subject_jwks(config: AuthConfig) -> dict[str, Any]: - ... - -def _select_subject_signing_key(subject_token: str, jwks: dict[str, Any]) -> Any: - ... +_jwks_clients: dict[str, jwt.PyJWKClient] = {} + +def _subject_jwks_client(config: AuthConfig) -> jwt.PyJWKClient: + jwks_uri = config.oidc.workload_subject_jwks_uri + if not jwks_uri: + raise jwt.InvalidTokenError("JWT subject token validation is disabled") + if jwks_uri not in _jwks_clients: + _jwks_clients[jwks_uri] = jwt.PyJWKClient(jwks_uri) + return _jwks_clients[jwks_uri]🤖 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 `@services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py` around lines 98 - 127, Replace the per-request `_fetch_subject_jwks` and manual filtering in `_select_subject_signing_key` with a cached `jwt.PyJWKClient` for `config.oidc.workload_subject_jwks_uri`; retain the disabled-validation error, use `get_signing_key_from_jwt(subject_token)` for key selection, and translate PyJWT/network failures into the existing `jwt.InvalidTokenError` handling contract.packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py (1)
165-197: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winWorkload provider isn't cached like the OIDC path.
_create_workload_exchange_providerbuilds a newWorkloadTokenExchangeProvider(and triggers discovery) on every_resolve_bootstrapcall, unlike the OIDC branch which reuses providers via_get_or_create_provider. Multiple clients in the same process will each perform redundant discovery/exchange calls.Also applies to: 452-456
🤖 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/nemo_platform_ext/src/nemo_platform_ext/client/factory.py` around lines 165 - 197, Cache the workload exchange provider in the same way as the OIDC provider: update _resolve_bootstrap and the provider cache flow around _get_or_create_provider so repeated resolutions reuse an existing WorkloadTokenExchangeProvider instead of calling _create_workload_exchange_provider and discovery each time. Include cache identity inputs such as the base URL and subject-token file, and preserve recreation when those configuration values change.openapi/ga/openapi.yaml (1)
12677-12688: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winIdentical
workload_identity_token_*fields duplicated across Kubernetes and Volcano profiles.Same two fields, same descriptions/defaults, copy-pasted between
KubernetesJobExecutionProfileConfigandVolcanoJobExecutionProfileConfig. Consider a shared base schema in the source Pydantic models to avoid drift.Also applies to: 18863-18874
🤖 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 `@openapi/ga/openapi.yaml` around lines 12677 - 12688, The workload identity token fields are duplicated in the Kubernetes and Volcano execution profile schemas. Extract workload_identity_token_expiration_seconds and workload_identity_token_audience into a shared base Pydantic schema, then have KubernetesJobExecutionProfileConfig and VolcanoJobExecutionProfileConfig inherit or reference it so the generated OpenAPI definitions remain consistent.
🤖 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 @.github/workflows/ci.yaml:
- Around line 1080-1092: Gate the “Pull auth-idp Kubernetes test image” step on
needs.build-cpu-smoke-images.outputs.publish_images == 'true', matching the
existing GHCR login condition, so docker pull runs only when the image was
published; alternatively ensure the image is published before this step.
In `@contrib/auth/authentik/kubernetes/nemo.yaml`:
- Around line 43-82: Add pod- and container-level securityContext settings to
the nemo Deployment, following the hardened patterns in envoy-gateway.yaml and
postgres.yaml: run as a non-root user, disallow privilege escalation, drop all
capabilities, and enable readOnlyRootFilesystem. Add any writable volume mounts
required for runtime data.
In `@openapi/ga/openapi.yaml`:
- Around line 10025-10028: Mark DockerWorkloadIdentityConfig.client_secret as
write-only at the source Pydantic model rather than editing the generated
OpenAPI file, using SecretStr or Field metadata that emits format: password and
writeOnly: true. Regenerate the OpenAPI schema and verify the property under
DockerJobExecutionProfileConfig is no longer exposed in GET responses.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 46-68: token_exchange_grant currently posts tokens without
validating transport security. Before calling httpx.post in
token_exchange_grant, apply the existing OIDC endpoint validation used by the
client implementation and reject any token_endpoint that is not HTTPS, including
HTTP URLs; preserve secure endpoint handling and raise the project’s established
validation error.
In `@packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py`:
- Around line 277-315: Validate the successful response payload in
token_exchange_grant and _exchange before accessing access_token. Reuse or
mirror the sibling helpers _response_json_object and _access_token_from_response
to require a JSON object containing a valid access_token, and raise
WorkloadTokenExchangeError for malformed or missing data instead of allowing
KeyError or AttributeError.
In `@services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py`:
- Around line 35-36: The module-level `_private_key` must not be regenerated on
each process start; load the workload signing key from a shared secret or
durable key store while keeping `_public_key` and the `kid` consistent across
replicas and restarts. Also update the subject JWKS retrieval used by the
workload token exchange flow to cache the fetched keys with an appropriate
refresh or expiry policy, avoiding an IdP request on every exchange.
In `@services/core/jobs/tests/controllers/test_docker_backend.py`:
- Around line 732-772: Make cleanup in
test_docker_job_injects_workload_identity_volume_when_token_exchange_enabled
exception-safe by wrapping the assertions and refresher lookup in a try/finally
block, with each workload identity refresher’s stop() called in the finally
block; alternatively, use a fixture finalizer to guarantee cleanup even when an
assertion fails.
- Around line 1698-1708: Update the stale comment above the volume assertions to
state that successful containers use three volumes per task instead of two;
leave the existing assertions and count unchanged.
- Around line 2567-2571: Update the stale comment above the volume assertions in
the multi-step job cleanup test to state that three volumes per task are
verified, matching the three volume names asserted and the expected call count.
---
Nitpick comments:
In `@contrib/auth/authentik/kubernetes/README.md`:
- Around line 1-92: Add a “Next Steps” section at the end of the README after
the k8s-test instructions, linking to the related Authentik Compose demo and the
authentication deployment/configuration documentation using the repository’s
correct relative Markdown paths.
In `@contrib/auth/authentik/kubernetes/redis.yaml`:
- Around line 27-37: Add readOnlyRootFilesystem: true to the Redis pod’s
securityContext alongside runAsNonRoot and allowPrivilegeEscalation settings,
while keeping the existing /data volumeMount writable for Redis data.
In `@contrib/auth/authentik/run.sh`:
- Line 247: Update run_k8s_tests and the related kustomization setup to derive
NMP_AUTHENTIK_K8S_KUSTOMIZATION from the same
${COMPOSE_DIR}/.generated/kubernetes expression used by k8s_render, instead of
hardcoding the repository path. Reuse the existing output_dir or a shared
variable so both code paths remain consistent if COMPOSE_DIR changes.
In `@openapi/ga/openapi.yaml`:
- Around line 12677-12688: The workload identity token fields are duplicated in
the Kubernetes and Volcano execution profile schemas. Extract
workload_identity_token_expiration_seconds and workload_identity_token_audience
into a shared base Pydantic schema, then have
KubernetesJobExecutionProfileConfig and VolcanoJobExecutionProfileConfig inherit
or reference it so the generated OpenAPI definitions remain consistent.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py`:
- Around line 165-197: Cache the workload exchange provider in the same way as
the OIDC provider: update _resolve_bootstrap and the provider cache flow around
_get_or_create_provider so repeated resolutions reuse an existing
WorkloadTokenExchangeProvider instead of calling
_create_workload_exchange_provider and discovery each time. Include cache
identity inputs such as the base URL and subject-token file, and preserve
recreation when those configuration values change.
In `@packages/nmp_common/src/nmp/common/config/base.py`:
- Around line 198-212: Enforce the documented dependency between BaseConfig
fields workload_subject_jwks_uri and workload_subject_issuers: add a model-level
validator that rejects configurations where workload_subject_jwks_uri is set
while workload_subject_issuers is empty, with a clear validation error. Preserve
valid configurations where the URI is unset or issuers are provided, and ensure
the validator matches the project’s existing Pydantic validation style.
In `@services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py`:
- Around line 98-127: Replace the per-request `_fetch_subject_jwks` and manual
filtering in `_select_subject_signing_key` with a cached `jwt.PyJWKClient` for
`config.oidc.workload_subject_jwks_uri`; retain the disabled-validation error,
use `get_signing_key_from_jwt(subject_token)` for key selection, and translate
PyJWT/network failures into the existing `jwt.InvalidTokenError` handling
contract.
🪄 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: 87388497-9cf5-4b55-8704-c08cf4a63898
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (78)
.github/workflows/ci.yamlMakefilecontrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/nemo.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/test_discovery.pyservices/core/auth/tests/test_workload_token_exchange.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/core/jobs/tests/test_config.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/authentik_live.pytests/auth_idp/providers.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.pytools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
💤 Files with no reviewable changes (4)
- Makefile
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
✅ Files skipped from review due to trivial changes (4)
- contrib/auth/authentik/kubernetes/namespace.yaml
- contrib/auth/authentik/.gitignore
- docs/auth/authentication/idp-integration.mdx
- docs/auth/deployment/configuration.mdx
🚧 Files skipped from review as they are similar to previous changes (23)
- tests/auth_idp/test_docs_links.py
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- contrib/auth/manifest.schema.yaml
- e2e/services_pool.py
- services/core/jobs/tests/test_config.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- contrib/auth/authentik/kubernetes/authentik.yaml
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- services/core/jobs/jobs-launcher/cmd/run.go
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- docs/auth/deployment/credential-propagation.mdx
- services/core/jobs/tests/controllers/test_workload_tokens.py
- contrib/auth/authentik/README.md
- e2e/conftest.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- packages/nemo_platform_ext/tests/config/test_config.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
Contributor
|
If there's something wrong with the way the helm chart installs the api container, please change the helm chart. I don't see why we are going to all this trouble to install authentik in what seems like an unusual way in addition to an apparently unusual install of the api for a test like this. There must be more context I don't know here? |
ironcommit
force-pushed
the
auth-idp-3/rsadler
branch
from
ba745d8 to
a7f6c62
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
openapi/ga/openapi.yaml (1)
10008-10057: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
client_secretstill notwriteOnly— leaks via job-profile GET.Same issue flagged previously:
DockerWorkloadIdentityConfig.client_secretlacksformat: password/writeOnly: true, unlike other secrets in this spec. It's reachable viaGET /apis/jobs/v2/execution-profilesthroughDockerJobExecutionProfileConfig.workload_identity. Fix in the source Pydantic model (this file is generated).🤖 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 `@openapi/ga/openapi.yaml` around lines 10008 - 10057, Mark DockerWorkloadIdentityConfig.client_secret as a write-only password field in the source Pydantic model, using the project’s established secret-field annotation pattern (format: password and writeOnly: true). Regenerate openapi/ga/openapi.yaml and verify DockerJobExecutionProfileConfig workload_identity no longer exposes client_secret in GET schemas.packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py (1)
47-69: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winStill no HTTPS check before
httpx.post(token_endpoint, ...).Prior review flagged this; no "addressed" marker and the code is unchanged —
token_endpointfrom discovery is posted to without scheme validation, risking cleartext subject-token leakage overhttp://. Add the same endpoint check used innemo_platform_plugin/client/oidc.pybefore the request.🤖 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/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py` around lines 47 - 69, Validate token_endpoint uses HTTPS before calling httpx.post in token_exchange_grant, matching the endpoint check implemented in nemo_platform_plugin/client/oidc.py; reject non-HTTPS schemes before transmitting the subject token, using the same error behavior and message style.
🧹 Nitpick comments (2)
contrib/auth/authentik/kubernetes/redis.yaml (1)
27-34: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider readOnlyRootFilesystem for the redis container.
Only
/datais written (already mounted); enablingreadOnlyRootFilesystem: trueshould be safe here, unlike postgres which needs its socket dir.🔒️ Proposed hardening
securityContext: runAsNonRoot: true runAsUser: 999 allowPrivilegeEscalation: false + readOnlyRootFilesystem: true capabilities: drop: ["ALL"] seccompProfile: type: RuntimeDefault🤖 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 `@contrib/auth/authentik/kubernetes/redis.yaml` around lines 27 - 34, Add readOnlyRootFilesystem: true to the Redis container’s securityContext alongside the existing hardening settings, ensuring the existing /data volume remains writable.Source: Linters/SAST tools
openapi/ga/individual/platform.openapi.yaml (1)
58-71: 🎯 Functional Correctness | 🔵 TrivialToken exchange response has no schema.
POST /apis/auth/tokendocumentsschema: {}for its 200 response — noaccess_token/token_type/expires_inshape, unlike the rest of the auth endpoints. Worth adding a response model in the source handler so generated SDK clients get a typed token response.🤖 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 `@openapi/ga/individual/platform.openapi.yaml` around lines 58 - 71, Add a typed response model for the token exchange handler, including access_token, token_type, and expires_in, and reference it in the 200 response schema for POST /apis/auth/token so the generated OpenAPI specification and SDK clients expose the token response shape.
🤖 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 `@openapi/ga/individual/platform.openapi.yaml`:
- Around line 10008-10057: Mark DockerWorkloadIdentityConfig.client_secret as a
secret by adding format: password and writeOnly: true, matching the treatment of
other secret fields in the OpenAPI specification.
In `@openapi/openapi.yaml`:
- Around line 10008-10057: The DockerWorkloadIdentityConfig.client_secret schema
is exposed as readable output. Update the source Pydantic model defining
DockerWorkloadIdentityConfig so client_secret uses a password format and is
write-only, then regenerate the OpenAPI schemas to propagate the change to both
specifications and prevent exposure through execution-profile GET responses.
In `@packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py`:
- Around line 215-221: Restrict the HTTP exception in _validate_token_endpoint
to loopback/local development endpoints instead of accepting every http:// URL.
When allow_http is enabled, parse and validate the hostname against
localhost/loopback addresses (IPv4 and IPv6), rejecting remote hosts; update
callers that derive allow_http from base_url so it cannot enable cleartext
exchange for arbitrary remote endpoints.
In
`@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py`:
- Line 68: The token request in the workload token controller must reject
non-HTTPS endpoints before sending credentials. Validate self.token_endpoint in
the method containing httpx.post, allowing only https:// (and, if explicitly
required for local development, loopback HTTP), then raise a clear configuration
error otherwise; update the internal IdP Docker configuration to use TLS rather
than non-loopback HTTP.
---
Duplicate comments:
In `@openapi/ga/openapi.yaml`:
- Around line 10008-10057: Mark DockerWorkloadIdentityConfig.client_secret as a
write-only password field in the source Pydantic model, using the project’s
established secret-field annotation pattern (format: password and writeOnly:
true). Regenerate openapi/ga/openapi.yaml and verify
DockerJobExecutionProfileConfig workload_identity no longer exposes
client_secret in GET schemas.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 47-69: Validate token_endpoint uses HTTPS before calling
httpx.post in token_exchange_grant, matching the endpoint check implemented in
nemo_platform_plugin/client/oidc.py; reject non-HTTPS schemes before
transmitting the subject token, using the same error behavior and message style.
---
Nitpick comments:
In `@contrib/auth/authentik/kubernetes/redis.yaml`:
- Around line 27-34: Add readOnlyRootFilesystem: true to the Redis container’s
securityContext alongside the existing hardening settings, ensuring the existing
/data volume remains writable.
In `@openapi/ga/individual/platform.openapi.yaml`:
- Around line 58-71: Add a typed response model for the token exchange handler,
including access_token, token_type, and expires_in, and reference it in the 200
response schema for POST /apis/auth/token so the generated OpenAPI specification
and SDK clients expose the token response shape.
🪄 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: 2dccdd37-71c8-4da8-9d74-cda6209ac08f
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (82)
.github/workflows/ci.yamlMakefilecontrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/nemo.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/config.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nemo_platform_plugin/tests/test_config.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/config/__init__.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/test_discovery.pyservices/core/auth/tests/test_workload_token_exchange.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/core/jobs/tests/test_config.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/authentik_live.pytests/auth_idp/providers.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.pytools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
💤 Files with no reviewable changes (4)
- tests/auth_idp/test_authentik_real_oidc.py
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- Makefile
- services/core/jobs/jobs-launcher/cmd/run_test.go
✅ Files skipped from review due to trivial changes (5)
- contrib/auth/authentik/.gitignore
- contrib/auth/authentik/kubernetes/namespace.yaml
- services/core/auth/src/nmp/core/auth/service.py
- docs/auth/deployment/configuration.mdx
- docs/set-up/config-reference.mdx
🚧 Files skipped from review as they are similar to previous changes (46)
- contrib/auth/manifest.schema.yaml
- contrib/auth/authentik/gateway/envoy.yaml
- contrib/auth/authentik/kustomization.yaml
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- tests/auth_idp/test_provider_layout.py
- docs/auth/authentication/idp-integration.mdx
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- tests/auth_idp/providers.py
- tests/auth_idp/test_docs_links.py
- contrib/auth/authentik/manifest.yaml
- tests/auth_idp/authentik_live.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- e2e/services_pool.py
- services/core/jobs/tests/test_config.py
- tests/auth_idp/test_fixture_helpers.py
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- tools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
- contrib/auth/authentik/kubernetes/envoy-gateway.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- tests/auth_idp/test_authentik_blueprint.py
- contrib/auth/authentik/blueprints/nemo.yaml
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- packages/nmp_common/src/nmp/common/auth/middleware.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.py
- contrib/auth/authentik/kubernetes/authentik.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- services/core/auth/tests/test_discovery.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- docs/auth/deployment/credential-propagation.mdx
- packages/nemo_platform_ext/tests/config/test_config.py
- contrib/auth/authentik/docker-compose.yml
- tests/auth_idp/test_provider_manifest.py
- services/core/jobs/jobs-launcher/cmd/run.go
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py
- .github/workflows/ci.yaml
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- e2e/conftest.py
- services/core/jobs/tests/controllers/test_workload_tokens.py
- contrib/auth/authentik/README.md
- contrib/auth/authentik/run.sh
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
Comment on lines
+215
to
+221
| def _validate_token_endpoint(token_endpoint: str, *, allow_http: bool = False) -> None: | ||
| """Reject non-HTTPS token endpoints (except loopback for local dev).""" | ||
| parsed = urlparse(token_endpoint) | ||
| if parsed.scheme == "https": | ||
| return | ||
| if allow_http and parsed.scheme == "http": | ||
| return |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find where allow_http is sourced/enabled
rg -nP --type=py -C3 '\ballow_http\b'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 163
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file and inspect the relevant region
wc -l packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py
sed -n '180,260p' packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py
# Find all references to allow_http and token endpoint validation
rg -n -C3 --hidden --glob '*.py' '\ballow_http\b|_validate_token_endpoint|token_endpoint' packages/nemo_platform_plugin/src/nemo_platform_pluginRepository: NVIDIA-NeMo/nemo-platform
Length of output: 26636
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the workload exchange path and how base_url is derived
sed -n '1,320p' packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
# Find every base_url source and every WorkloadTokenExchangeProvider instantiation
rg -n -C4 --hidden --glob '*.py' '\bbase_url\b|WorkloadTokenExchangeProvider\(' packages/nemo_platform_plugin/src/nemo_platform_pluginRepository: NVIDIA-NeMo/nemo-platform
Length of output: 50381
Keep allow_http loopback-only
allow_http is wired to any http:// base_url, not just localhost, so workload token exchange can post the subject token over cleartext to a remote token endpoint. Keep the loopback/private-host restriction here, or gate allow_http to local dev only.
🤖 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/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py` around
lines 215 - 221, Restrict the HTTP exception in _validate_token_endpoint to
loopback/local development endpoints instead of accepting every http:// URL.
When allow_http is enabled, parse and validate the hostname against
localhost/loopback addresses (IPv4 and IPv6), rejecting remote hosts; update
callers that derive allow_http from base_url so it cannot enable cleartext
exchange for arbitrary remote endpoints.
Source: Linters/SAST tools
| if self.scope: | ||
| data["scope"] = self.scope | ||
|
|
||
| response = httpx.post(self.token_endpoint, data=data, timeout=self.timeout) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Require HTTPS for password-grant endpoints.
This posts password and optional client_secret; current Docker configuration explicitly accepts non-loopback http:// endpoints, exposing credentials to any network-path observer. Reject HTTP and configure the internal IdP with TLS.
🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py`
at line 68, The token request in the workload token controller must reject
non-HTTPS endpoints before sending credentials. Validate self.token_endpoint in
the method containing httpx.post, allowing only https:// (and, if explicitly
required for local development, loopback HTTP), then raise a clear configuration
error otherwise; update the internal IdP Docker configuration to use TLS rather
than non-loopback HTTP.
c972546 to
7241d9d
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 `@packages/nemo_platform_ext/tests/client/test_client.py`:
- Around line 203-219: Mock discover_nmp_config in
test_env_access_token_takes_precedence_over_workload_identity_file so client
creation does not perform real OIDC discovery against the configured base URL.
Configure the mock with the expected discovery result while preserving the
existing environment variables and Authorization-header assertion.
🪄 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: cf43b411-1967-47a8-9473-6b9b006e91a3
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (82)
.github/workflows/ci.yamlMakefilecontrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/nemo.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/config.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nemo_platform_plugin/tests/test_config.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/config/__init__.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/test_discovery.pyservices/core/auth/tests/test_workload_token_exchange.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/core/jobs/tests/test_config.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/authentik_live.pytests/auth_idp/providers.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.pytools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
💤 Files with no reviewable changes (4)
- tests/auth_idp/test_authentik_real_oidc.py
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- Makefile
- services/core/jobs/jobs-launcher/cmd/run_test.go
✅ Files skipped from review due to trivial changes (5)
- contrib/auth/authentik/.gitignore
- contrib/auth/authentik/kustomization.yaml
- packages/nmp_common/src/nmp/common/config/init.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.py
- docs/set-up/config-reference.mdx
🚧 Files skipped from review as they are similar to previous changes (53)
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- contrib/auth/manifest.schema.yaml
- tests/auth_idp/test_docs_links.py
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- packages/nmp_common/src/nmp/common/auth/middleware.py
- contrib/auth/authentik/blueprints/nemo.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- docs/auth/authentication/idp-integration.mdx
- tests/auth_idp/test_fixture_helpers.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- tools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
- contrib/auth/authentik/kubernetes/namespace.yaml
- services/core/auth/src/nmp/core/auth/service.py
- services/core/jobs/tests/test_config.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- contrib/auth/authentik/kubernetes/envoy-gateway.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- docs/auth/deployment/configuration.mdx
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- tests/auth_idp/test_authentik_blueprint.py
- services/core/jobs/jobs-launcher/cmd/run.go
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.py
- contrib/auth/authentik/kubernetes/nemo.yaml
- contrib/auth/authentik/kubernetes/authentik.yaml
- e2e/services_pool.py
- services/core/auth/tests/test_discovery.py
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- contrib/auth/authentik/manifest.yaml
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- tests/auth_idp/test_provider_layout.py
- tests/auth_idp/test_provider_manifest.py
- packages/nemo_platform_plugin/tests/test_config.py
- tests/auth_idp/authentik_live.py
- packages/nmp_common/src/nmp/common/config/base.py
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- packages/nemo_platform_ext/tests/config/test_config.py
- contrib/auth/authentik/docker-compose.yml
- tests/auth_idp/providers.py
- docs/auth/deployment/credential-propagation.mdx
- contrib/auth/authentik/README.md
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py
- contrib/auth/authentik/run.sh
- packages/nemo_platform_plugin/src/nemo_platform_plugin/config.py
- e2e/conftest.py
- services/core/jobs/tests/controllers/test_workload_tokens.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
ironcommit
force-pushed
the
auth-idp-3/rsadler
branch
from
7241d9d to
92efa2d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contrib/auth/authentik/kubernetes/README.md (1)
88-92: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd a
Next Stepssection with links to the Compose demo and workload-auth documentation.As per coding guidelines, “Include 'Next Steps' section at the end with cross-links to related documentation content.”
🤖 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 `@contrib/auth/authentik/kubernetes/README.md` around lines 88 - 92, Add a Next Steps section at the end of the README, including links to the Compose demo and workload-auth documentation. Preserve the existing k8s-test and runtime instructions unchanged.Source: Coding guidelines
🧹 Nitpick comments (3)
packages/nemo_platform_ext/tests/auth/test_workload_exchange.py (1)
24-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a concrete annotation for
claims.-def _make_jwt(claims: dict) -> str: +def _make_jwt(claims: dict[str, object]) -> str:As per coding guidelines, “prefer concrete type hints over string-based type hints.”
🤖 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/nemo_platform_ext/tests/auth/test_workload_exchange.py` around lines 24 - 29, Update the claims parameter annotation in _make_jwt to use a concrete mapping type that reflects the JSON object expected by the JWT payload, rather than the overly generic dict annotation. Preserve the function’s existing behavior and return type.Source: Coding guidelines
openapi/ga/individual/platform.openapi.yaml (2)
10045-10055: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo minimum bound on
subject_token_ttl_seconds/refresh_margin_seconds.Unlike the analogous Kubernetes field (
workload_identity_token_expiration_seconds,minimum: 600), these Docker fields accept any integer, including 0 or negative, which would produce nonsensical refresh/expiry behavior.Fix belongs in the source Pydantic model backing
DockerWorkloadIdentityConfig; please confirm whether it enforces a floor at runtime even though the schema doesn't.🤖 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 `@openapi/ga/individual/platform.openapi.yaml` around lines 10045 - 10055, Update the source Pydantic model backing DockerWorkloadIdentityConfig to enforce a minimum of 600 seconds for both subject_token_ttl_seconds and refresh_margin_seconds, matching the analogous Kubernetes field. Ensure the runtime validation rejects zero and negative values, then regenerate the OpenAPI schema so both properties expose the same minimum constraint.
58-71: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winToken exchange endpoint has no documented response schema.
POST /apis/auth/token200 response isschema: {}— no fields documented for a new, security-sensitive RFC 8693 endpoint. SDK/client consumers get no contract foraccess_token/token_type/expires_in.Add a response model to the underlying FastAPI route (this file is generated, so the fix belongs in source).
🤖 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 `@openapi/ga/individual/platform.openapi.yaml` around lines 58 - 71, Update the source FastAPI route for POST /apis/auth/token, identified by operationId token_exchange_apis_auth_token_post, to declare an explicit response model covering the RFC 8693 token response fields access_token, token_type, and expires_in; regenerate this OpenAPI file so the 200 response references that model instead of schema: {}.
🤖 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 `@contrib/auth/authentik/kubernetes/README.md`:
- Line 1: Replace every hardcoded “NeMo” product-name occurrence in the README,
including the referenced sections, with the established Sphinx documentation
substitution syntax. Preserve the surrounding Markdown text and formatting while
ensuring all product references consistently use substitutions rather than
literal names.
In `@contrib/auth/authentik/kubernetes/workload-token-signing-key.yaml`:
- Around line 9-37: Remove the committed PEM private key from the
workload-token-signing-key manifest and purge it from repository history. Rotate
the corresponding public key and configuration references, then provision
replacement demo credentials out-of-band or generate them during setup rather
than committing usable signing material.
In `@openapi/openapi.yaml`:
- Around line 10045-10055: Update the source model definitions for
subject_token_ttl_seconds and refresh_margin_seconds to enforce minimum values
of 1 and 0 respectively, then regenerate openapi.yaml so the generated schema
reflects these constraints.
- Around line 58-71: Update the POST operation for
token_exchange_apis_auth_token_post to define an
application/x-www-form-urlencoded request schema containing the RFC 8693
token-exchange fields, and replace the empty success schema with a structured
token response model covering the returned access-token data. Reuse existing
component schemas where available and preserve the endpoint’s current response
status and description.
In `@packages/nemo_platform_plugin/tests/test_client_auth.py`:
- Around line 298-309: Update token exchange HTTP validation in
token_exchange_grant and the related tests around the platform URL/discovered
endpoint cases. Require HTTPS for non-loopback token endpoints, and permit HTTP
only when the endpoint host is an explicit loopback address such as localhost or
127.0.0.1; do not let allow_http on the platform URL authorize unrelated
discovered endpoints. Update the affected test fixtures and assertions to cover
these endpoint-specific rules.
---
Outside diff comments:
In `@contrib/auth/authentik/kubernetes/README.md`:
- Around line 88-92: Add a Next Steps section at the end of the README,
including links to the Compose demo and workload-auth documentation. Preserve
the existing k8s-test and runtime instructions unchanged.
---
Nitpick comments:
In `@openapi/ga/individual/platform.openapi.yaml`:
- Around line 10045-10055: Update the source Pydantic model backing
DockerWorkloadIdentityConfig to enforce a minimum of 600 seconds for both
subject_token_ttl_seconds and refresh_margin_seconds, matching the analogous
Kubernetes field. Ensure the runtime validation rejects zero and negative
values, then regenerate the OpenAPI schema so both properties expose the same
minimum constraint.
- Around line 58-71: Update the source FastAPI route for POST /apis/auth/token,
identified by operationId token_exchange_apis_auth_token_post, to declare an
explicit response model covering the RFC 8693 token response fields
access_token, token_type, and expires_in; regenerate this OpenAPI file so the
200 response references that model instead of schema: {}.
In `@packages/nemo_platform_ext/tests/auth/test_workload_exchange.py`:
- Around line 24-29: Update the claims parameter annotation in _make_jwt to use
a concrete mapping type that reflects the JSON object expected by the JWT
payload, rather than the overly generic dict annotation. Preserve the function’s
existing behavior and return type.
🪄 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: c01251da-96f1-4e76-9485-46057f39b07d
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (84)
.github/actions/setup-kind-cluster/action.yaml.github/workflows/ci.yamlMakefilecontrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/nemo.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kubernetes/workload-token-signing-key.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/config.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nemo_platform_plugin/tests/test_config.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/config/__init__.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/test_discovery.pyservices/core/auth/tests/test_workload_token_exchange.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/core/jobs/tests/test_config.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/authentik_live.pytests/auth_idp/providers.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.pytools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
💤 Files with no reviewable changes (4)
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- Makefile
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
🚧 Files skipped from review as they are similar to previous changes (54)
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.py
- contrib/auth/authentik/.gitignore
- contrib/auth/authentik/kubernetes/namespace.yaml
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- contrib/auth/authentik/kustomization.yaml
- packages/nemo_platform_plugin/tests/test_config.py
- tests/auth_idp/authentik_live.py
- services/core/auth/src/nmp/core/auth/service.py
- packages/nmp_common/src/nmp/common/config/init.py
- services/core/auth/tests/test_discovery.py
- tests/auth_idp/test_authentik_blueprint.py
- e2e/services_pool.py
- services/core/jobs/tests/test_config.py
- contrib/auth/authentik/kubernetes/envoy-gateway.yaml
- contrib/auth/authentik/kubernetes/nemo.yaml
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- contrib/auth/manifest.schema.yaml
- packages/nmp_common/src/nmp/common/auth/middleware.py
- contrib/auth/authentik/blueprints/nemo.yaml
- docs/auth/deployment/configuration.mdx
- tools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
- contrib/auth/authentik/kubernetes/authentik.yaml
- contrib/auth/authentik/docker-compose.yml
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- contrib/auth/authentik/manifest.yaml
- docs/auth/authentication/idp-integration.mdx
- tests/auth_idp/test_provider_layout.py
- packages/nmp_common/src/nmp/common/config/base.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- contrib/auth/authentik/gateway/envoy.yaml
- tests/auth_idp/test_fixture_helpers.py
- packages/nemo_platform_ext/tests/config/test_config.py
- docs/set-up/config-reference.mdx
- services/core/jobs/tests/controllers/test_workload_tokens.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- docs/auth/deployment/credential-propagation.mdx
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- services/core/jobs/jobs-launcher/cmd/run.go
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- tests/auth_idp/test_docs_links.py
- services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py
- tests/auth_idp/providers.py
- contrib/auth/authentik/README.md
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/config.py
- e2e/conftest.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- contrib/auth/authentik/run.sh
Comment on lines
+9
to
+37
| private-key.pem: | | ||
| -----BEGIN PRIVATE KEY----- | ||
| MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQCOedG8lq63ZW2u | ||
| W8tmIDuk0AS/yGa7wbWSZVV5kcqXBUVTFrr/BobIxlp+QTINXnflWl7MG33AoP3T | ||
| hzOIHZ3nVo56rmvQMAzrxOhTtQLEA5uH++wjj8M/KBo/2/504jT3wA89xCkGGjVF | ||
| p1XgTr4a2+oOdtY9rywrSpmuRb0aenLbK3ZSpcGorkeAXlslCsC0YkMxQHnFFcV1 | ||
| 6v+pPsKo5kPANsVW1V4ChHe6eV7vs8FMD3IJrsN7ejNr9tRyYXpt7WwwXTZfrJgN | ||
| idWYO8fGURR2au3cNi7TZNgc2KFyUfmZmz2XSw+bhi7ljt3IleQbrJmwqxgGbX7c | ||
| 8yM7V14xAgMBAAECggEAFaz12jWodhFRyvV/L9XWjR/yoQTJkGYLjC6rftfN1QUB | ||
| w37RSyxAhDodGVks1rjZD8UcZ/wEuTIk+7H1ho9O7xuNiJ+RmnY8+P+Xguimd5C+ | ||
| 8ETE67Ia97hPqgh7oWeqvEh1aSW3QgzpruOi3oHMTUTIypOMy4yjcSCj2c4VU3o0 | ||
| aTJExPhOfgb/9+/2aSFHtlqVlenCgAwSi2gHCSovmucPxaVlw0Iv+Ko+4VzZ3aK4 | ||
| AOo7xx+ENYcO7zQJhBJHt0xZH5+0re3Ln4Y1RcxKntq9xuhFd3eGMEEp0B0GoUPf | ||
| ZN87mPRR2NxRtGNmWbJnErDcjIqU5f99j+KCSxgcuwKBgQDGTpOG+90GSXl0rSwJ | ||
| ifEbn8jA12FPQT+jDiDo03CF9l/1Q89L60uluX43GOPqgYf3wSLo8ngwpJrAsD6a | ||
| RoYknczVbVpxM8+RA+NSOMELlG+z9qzeEcLOawNfaeledqUx33lUA1NoyKtewfPF | ||
| 8aUEFLcq2HWAr3EQgA8CAZLF/wKBgQC37RQNZjAGwpjrp7DXA7/mOq6XLJVtdOQt | ||
| OnG2+5Gl8+qeAuBlhHHZLf7BBRwhjKVfBf6bNWZ52xBf0P4i+7ua/ojY5zfO+dxL | ||
| XZAnoiUVPA0wcWoD73MdqA92H9EWHr930WDr5Ze7b8fWRS/gv/RiwLlywzqZeAVQ | ||
| OQUyEPi7zwKBgQCGn9NhYNy0pu6wOdvage3P7hJwcAccsdQM0sueo+QQ3ahZLfat | ||
| uIf7csjviJM5oUP3+wLDF9BmXWi9hSiKvXPVqdbj0UAD98Ni1Dvflq3fOZ73km/a | ||
| tj4EuftpK9KFEZCmIesREMzwJsLIKm1epOR8t799c8OZYAEmzXjiyMSmawKBgQC1 | ||
| ed8HOYS5pthr7JG4FaMK72vFDeCCl4tsdPYurhoPvJQKRJxUjlInEBMFMJ2JHNUu | ||
| 5SNK0W3DXjrRz8xxiCCb1foAymNufWq3kiHieUJNqWr6GW2ZZHIZGPHog4wyXzRc | ||
| bONM3sTP1INBEBMll0DzD2XrrmXrd1kGxrwxoj1KUQKBgFxlop4lr1cZSXvPtSGG | ||
| iC7VObEGm4uKImdRJVPpCzcNYM3GrO7IBzpsvJvD7xzPKmKgAWOrbT+w2OEuNEYr | ||
| tID4nHisDEP95ggS/Ux+Um2ew47W0xgwyCWwrAq3o2UXqqipgv4QQOG4om5BrMxN | ||
| vbKoDMZBr2NX/RFdcxmEXccf | ||
| -----END PRIVATE KEY----- |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Remove and rotate the committed signing key.
This usable private signing key is exposed in source control and must be treated as compromised. Remove it from repository history, rotate the corresponding public key/configuration, and provision demo keys out-of-band or generate them during setup; the comment does not make this safe.
🧰 Tools
🪛 Betterleaks (1.6.1)
[high] 10-37: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
🪛 Checkov (3.3.8)
[medium] 9-10: Private Key
(CKV_SECRET_13)
🪛 GitHub Check: Secrets Scan
[warning] 10-10:
Found unverified PrivateKey result 🐷🔑
🤖 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 `@contrib/auth/authentik/kubernetes/workload-token-signing-key.yaml` around
lines 9 - 37, Remove the committed PEM private key from the
workload-token-signing-key manifest and purge it from repository history. Rotate
the corresponding public key and configuration references, then provision
replacement demo credentials out-of-band or generate them during setup rather
than committing usable signing material.
Source: Linters/SAST tools
Comment on lines
+298
to
+309
| def test_token_exchange_grant_allows_http_when_platform_base_url_is_http(self): | ||
| with patch("nemo_platform_plugin.client.oidc.httpx.post") as mock_post: | ||
| mock_post.return_value = httpx.Response(200, json={"access_token": "exchanged-token", "expires_in": 300}) | ||
|
|
||
| token_data = token_exchange_grant( | ||
| token_endpoint="http://nemo-gateway:8080/apis/auth/token", | ||
| client_id="nemo-platform-workload", | ||
| subject_token="subject-token", | ||
| allow_http=True, | ||
| ) | ||
|
|
||
| assert token_data["access_token"] == "exchanged-token" |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Restrict allow_http to loopback token endpoints.
These tests permit cleartext subject-token exchange with non-loopback hosts and make an HTTP platform URL authorize unrelated discovered token endpoints. Validate the token endpoint itself and allow HTTP only for explicit loopback development endpoints.
Also applies to: 502-555
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 302-302: Do not make http calls without encryption
Context: "http://nemo-gateway:8080/apis/auth/token"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
🤖 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/nemo_platform_plugin/tests/test_client_auth.py` around lines 298 -
309, Update token exchange HTTP validation in token_exchange_grant and the
related tests around the platform URL/discovered endpoint cases. Require HTTPS
for non-loopback token endpoints, and permit HTTP only when the endpoint host is
an explicit loopback address such as localhost or 127.0.0.1; do not let
allow_http on the platform URL authorize unrelated discovered endpoints. Update
the affected test fixtures and assertions to cover these endpoint-specific
rules.
Source: Linters/SAST tools
ironcommit
force-pushed
the
auth-idp-3/rsadler
branch
from
1b5fee7 to
4c07edc
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py (1)
47-68: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftStill no HTTPS enforcement on the password-grant token endpoint.
issue()postspassword/client_secrettoself.token_endpointwithout validating the scheme. Same concern as raised previously; not addressed in this revision. The SSRF static-analysis hint on this line is a false positive here (endpoint is operator/deployment-configured, not user-controlled), but the credentials-over-plaintext-HTTP risk stands.🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py` around lines 47 - 68, Add HTTPS scheme validation in the password-grant token issuance flow before the httpx.post call in issue(), rejecting any token_endpoint that is not HTTPS. Preserve the existing request behavior for valid HTTPS endpoints and ensure credentials are never sent over plaintext HTTP.
🧹 Nitpick comments (1)
openapi/ga/openapi.yaml (1)
37-57: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winJWKS response schema doesn't model an actual JWK Set.
/apis/auth/jwksreturnsadditionalProperties: {items: object, type: array}(an arbitrary dict-of-arrays) rather than the standard RFC 7517 shape ({"keys": [...]}with typed key fields likekty,use,n,e). This looks like it was generated from a loosely-typed Python return annotation (e.g.dict[str, list[dict]]). Third-party OIDC/JWKS clients validating against the standard shape may reject this.Type the FastAPI handler's return value with a proper JWKS Pydantic model (a
keys: list[JWK]field) instead of a raw dict, then regenerate this spec.🤖 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 `@openapi/ga/openapi.yaml` around lines 37 - 57, Update the FastAPI handler for /apis/auth/jwks to return a typed JWKS Pydantic model with a keys: list[JWK] field, and define JWK fields such as kty, use, n, and e with appropriate types. Replace the loosely typed dictionary return annotation, then regenerate openapi.yaml so the response schema reflects the standard RFC 7517 JWK Set shape.
🤖 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 `@contrib/auth/authentik/helm/templates/shared-postgres-initdb-configmap.yaml`:
- Around line 14-23: Update the PostgreSQL initialization command in the
ConfigMap so AUTHENTIK_PASSWORD and NEMO_PASSWORD are no longer passed through
psql --set arguments or otherwise exposed in argv. Provide both secrets through
protected stdin, a temporary protected file, or another non-command-line
mechanism while preserving the existing initialization behavior and variable
names.
In `@k8s/helm/ci/22-envoy-config-override.yaml`:
- Around line 16-20: Update the Envoy admin configuration under
admin.address.socket_address to bind address 127.0.0.1 instead of 0.0.0.0, while
leaving port_value 9901 unchanged.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 156-161: Reject exchanged tokens that are already expired before
caching or returning them: in
packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py lines
156-161, validate the constructed TokenSet with zero expiry margin and raise the
established rejection error; apply the same post-construction validation in
packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py lines
576-580. Update packages/nemo_platform_ext/tests/auth/test_workload_exchange.py
lines 145-169 to assert expired-token rejection instead of success.
---
Duplicate comments:
In
`@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py`:
- Around line 47-68: Add HTTPS scheme validation in the password-grant token
issuance flow before the httpx.post call in issue(), rejecting any
token_endpoint that is not HTTPS. Preserve the existing request behavior for
valid HTTPS endpoints and ensure credentials are never sent over plaintext HTTP.
---
Nitpick comments:
In `@openapi/ga/openapi.yaml`:
- Around line 37-57: Update the FastAPI handler for /apis/auth/jwks to return a
typed JWKS Pydantic model with a keys: list[JWK] field, and define JWK fields
such as kty, use, n, and e with appropriate types. Replace the loosely typed
dictionary return annotation, then regenerate openapi.yaml so the response
schema reflects the standard RFC 7517 JWK Set shape.
🪄 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: fa37dc9a-c652-45a5-b830-55379fdfba8b
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (99)
.github/actions/setup-kind-cluster/action.yaml.github/workflows/ci.yamlMakefilecontrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/helm/.helmignorecontrib/auth/authentik/helm/Chart.yamlcontrib/auth/authentik/helm/files/blueprints/nemo.yamlcontrib/auth/authentik/helm/templates/_helpers.tplcontrib/auth/authentik/helm/templates/blueprint-configmap.yamlcontrib/auth/authentik/helm/templates/shared-postgres-initdb-configmap.yamlcontrib/auth/authentik/helm/templates/shared-postgres-nemo-secret.yamlcontrib/auth/authentik/helm/templates/shared-postgres-secret.yamlcontrib/auth/authentik/helm/templates/shared-postgres-service.yamlcontrib/auth/authentik/helm/templates/shared-postgres-serviceaccount.yamlcontrib/auth/authentik/helm/templates/shared-postgres-statefulset.yamlcontrib/auth/authentik/helm/templates/tokenreview-rbac.yamlcontrib/auth/authentik/helm/templates/workload-token-signing-key-secret.yamlcontrib/auth/authentik/helm/templates/workload-token-tls.yamlcontrib/auth/authentik/helm/values.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyk8s/helm/README.mdk8s/helm/ci/21-api-extra-volumes.yamlk8s/helm/ci/22-envoy-config-override.yamlk8s/helm/templates/api/api-deployment.yamlk8s/helm/templates/platform-seed-job.yamlk8s/helm/templates/proxy/_helpers.tplk8s/helm/templates/proxy/envoy-configmap.yamlk8s/helm/templates/proxy/envoy-deployment.yamlk8s/helm/values.yamlopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/config.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nemo_platform_plugin/tests/test_config.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/config/__init__.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/test_discovery.pyservices/core/auth/tests/test_workload_token_exchange.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/core/jobs/tests/test_config.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/authentik_live.pytests/auth_idp/providers.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.pytools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
💤 Files with no reviewable changes (4)
- Makefile
- tests/auth_idp/test_authentik_real_oidc.py
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
🚧 Files skipped from review as they are similar to previous changes (50)
- contrib/auth/authentik/.gitignore
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.py
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- contrib/auth/manifest.schema.yaml
- docs/auth/authentication/idp-integration.mdx
- tests/auth_idp/test_authentik_blueprint.py
- packages/nmp_common/src/nmp/common/config/init.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- .github/actions/setup-kind-cluster/action.yaml
- tests/auth_idp/test_docs_links.py
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- services/core/auth/src/nmp/core/auth/service.py
- e2e/services_pool.py
- contrib/auth/authentik/manifest.yaml
- tests/auth_idp/test_fixture_helpers.py
- services/core/auth/tests/test_discovery.py
- contrib/auth/authentik/gateway/envoy.yaml
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- packages/nemo_platform_plugin/tests/test_config.py
- tools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
- contrib/auth/authentik/docker-compose.yml
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- tests/auth_idp/authentik_live.py
- services/core/jobs/jobs-launcher/cmd/run.go
- contrib/auth/authentik/blueprints/nemo.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- docs/auth/deployment/configuration.mdx
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- docs/auth/deployment/credential-propagation.mdx
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.py
- packages/nmp_common/src/nmp/common/config/base.py
- services/core/jobs/tests/controllers/test_workload_tokens.py
- e2e/conftest.py
- packages/nemo_platform_ext/tests/config/test_config.py
- tests/auth_idp/test_provider_layout.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/config.py
- tests/auth_idp/test_provider_manifest.py
- .github/workflows/ci.yaml
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- contrib/auth/authentik/README.md
- services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- tests/auth_idp/providers.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
- docs/set-up/config-reference.mdx
ironcommit
force-pushed
the
auth-idp-3/rsadler
branch
2 times, most recently
from
f0cb756 to
abed58d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
contrib/auth/authentik/helm/templates/shared-postgres-initdb-configmap.yaml (1)
18-27: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDB passwords still exposed in
psqlargv.
authentik_password/nemo_passwordare passed via--set=on the command line;ps/diagnostics can read them. Feed them through stdin variables instead (e.g. set them inside the heredoc via\setreading from:'PASSWORD'env, or pipe a\gsetblock), keeping secrets out of argv.🤖 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 `@contrib/auth/authentik/helm/templates/shared-postgres-initdb-configmap.yaml` around lines 18 - 27, Update the initdb psql invocation to stop passing authentik_password and nemo_password through --set command-line arguments; provide both secrets through the heredoc/stdin using psql variable handling, and preserve their use in the CREATE USER statements while keeping passwords out of process argv.services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py (1)
56-68: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winEnforce HTTPS before posting credentials.
issue()still POSTspassword/client_secrettoself.token_endpointwith no scheme check. Reject non-https://(allow loopback HTTP only if required for local dev) before the request.🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py` around lines 56 - 68, Update WorkloadToken’s issue method to validate self.token_endpoint before constructing or sending the credential-bearing httpx.post request, rejecting non-HTTPS endpoints while allowing HTTP only for explicitly permitted loopback development hosts. Preserve the existing token request flow for valid endpoints and fail clearly before transmitting credentials when validation fails.packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py (2)
47-69: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winMissing token-endpoint scheme validation — subject token posted to unvalidated URL.
token_exchange_grantpostssubject_tokenviahttpx.post(token_endpoint, ...)with no scheme check at all. The sibling implementation inoidc.pyadded_validate_token_endpointto reject non-HTTPS endpoints for this exact reason; that guard was never mirrored here, so anhttp://(or otherwise unvalidated)token_endpointcan leak the subject token in cleartext.🛡️ Proposed fix
+from urllib.parse import urlparse + + +def _validate_token_endpoint(token_endpoint: str, *, allow_http: bool = False) -> None: + parsed = urlparse(token_endpoint) + if parsed.scheme == "https": + return + if parsed.scheme == "http" and parsed.hostname in {"localhost", "127.0.0.1", "::1"}: + return + raise ValueError(f"Workload token endpoint must use HTTPS (got {token_endpoint!r}).") + + def token_exchange_grant( *, token_endpoint: str, ... ) -> dict[str, object]: """Execute RFC 8693 token exchange and return token response JSON.""" + _validate_token_endpoint(token_endpoint) data: dict[str, str] = {🤖 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/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py` around lines 47 - 69, Update token_exchange_grant to validate token_endpoint with the existing _validate_token_endpoint helper before calling httpx.post, rejecting non-HTTPS or otherwise invalid schemes while preserving the current token exchange request behavior.
146-161: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winExchanged token can be already-expired and still gets cached/returned.
_exchangebuildsTokenSet.from_access_tokenfrom whateverexpires_in/expthe IdP returns, with no check that the result isn't already expired. A misconfigured IdP (or clock skew) makes the very firstget_access_token()call succeed with a dead token, and callers only discover the failure on the next outbound request.🤖 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/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py` around lines 146 - 161, The _exchange method caches exchanged tokens without validating their expiration. After constructing the TokenSet, verify it is still valid using the existing token-expiration mechanism; reject or avoid caching an already-expired token so get_access_token does not return it.packages/nemo_platform_plugin/tests/test_client_auth.py (1)
298-309: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winTests lock in the non-loopback
allow_httpbypass.These tests assert the current permissive behavior (
allow_http=Trueaccepted forhttp://nemo-gateway:8080, non-loopback). Once_validate_token_endpoint's loopback restriction is restored, update these to expect rejection for non-loopback hosts.Also applies to: 502-556
🤖 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/nemo_platform_plugin/tests/test_client_auth.py` around lines 298 - 309, Update the token exchange tests around test_token_exchange_grant_allows_http_when_platform_base_url_is_http and the corresponding tests near the later covered range to expect rejection when allow_http=True is used with non-loopback HTTP endpoints such as nemo-gateway. Replace successful token assertions with validation of the appropriate error, while preserving successful behavior for loopback HTTP endpoints.packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py (1)
215-227: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
allow_http=Trueisn't restricted to loopback — enables cleartext exchange with any host.Lines 220-221 accept
http://for any hostname onceallow_http=True, bypassing the loopback-only restriction that the unconditional branch (222-223) still enforces. Ifallow_httpis derived from the platform's own base URL beinghttp://(as flagged previously), a cleartext platform deployment implicitly authorizes posting the subject token to any discoveredhttp://token endpoint, not just local dev.🛡️ Proposed fix
if parsed.scheme == "https": return - if allow_http and parsed.scheme == "http": - return if parsed.scheme == "http" and parsed.hostname in {"localhost", "127.0.0.1", "::1"}: return + if allow_http and parsed.scheme == "http" and parsed.hostname in {"localhost", "127.0.0.1", "::1"}: + return raise ValueError(🤖 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/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py` around lines 215 - 227, Restrict the HTTP acceptance logic in _validate_token_endpoint to loopback hostnames only, regardless of allow_http. Remove the allow_http branch that permits arbitrary HTTP hosts, while preserving HTTPS acceptance and the existing localhost, 127.0.0.1, and ::1 behavior.
🧹 Nitpick comments (4)
packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py (1)
47-161: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDuplicate exchange logic across
nemo_platform_extandnemo_platform_plugin.This file re-implements
token_exchange_grant/WorkloadTokenExchangeProvideralmost identically topackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py. The two have already drifted (this file is missing the HTTPS check the plugin has) — consider sharing one implementation to avoid future security-relevant divergence.🤖 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/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py` around lines 47 - 161, Replace the duplicated token exchange implementation in token_exchange_grant and WorkloadTokenExchangeProvider with the existing shared implementation from nemo_platform_plugin.client.oidc, or centralize it in a common module consumed by both packages. Preserve the current public behavior while ensuring security checks such as the plugin’s HTTPS validation remain consistently applied.contrib/auth/authentik/helm/templates/workload-token-tls.yaml (2)
14-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing common labels.
Other templates in this chart (statefulset, service) include
{{- include "nemo-authentik-demo.labels" . | nindent 4 }}in addition to the component label. This secret only setsapp.kubernetes.io/component, breaking label consistency across the chart's resources.♻️ Proposed fix
labels: + {{- include "nemo-authentik-demo.labels" . | nindent 4 }} app.kubernetes.io/component: workload-token-tls🤖 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 `@contrib/auth/authentik/helm/templates/workload-token-tls.yaml` around lines 14 - 19, Update the Secret metadata in the workload-token TLS template to include the chart’s shared labels via the existing nemo-authentik-demo.labels helper, while preserving the app.kubernetes.io/component label and current metadata fields.
10-11: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueCA/cert generated even when reusing an existing secret.
genCA/genSignedCertrun unconditionally before the$hasExistingTlscheck, wasting the generation even when the existing cert is reused.🤖 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 `@contrib/auth/authentik/helm/templates/workload-token-tls.yaml` around lines 10 - 11, Move the genCA and genSignedCert calls associated with $ca and $cert inside the branch that creates new TLS material, after the $hasExistingTls check. Ensure existing secrets reuse their certificate without invoking either generation function, while preserving the current generation inputs for new secrets.contrib/auth/manifest.schema.yaml (1)
68-94: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPlaintext
passwordfield alongsidepassword_env_var.Both
human_grantandmachine_grantpermit a rawpasswordstring directly in the manifest, in addition to the saferpassword_env_var. This lets credentials be committed to config files despite an env-var alternative already existing. Consider dropping the plaintextpasswordproperty (or restricting it toreference-only/local-dev mode) and requiringpassword_env_varexclusively; also note theanyOfdoesn't prevent supplying bothpasswordandpassword_env_varsimultaneously —oneOfwould enforce exclusivity.Also applies to: 95-121
🤖 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 `@contrib/auth/manifest.schema.yaml` around lines 68 - 94, The auth manifest schema permits plaintext password values and does not enforce exclusivity. Update both human_grant and machine_grant to remove the password property and require password_env_var exclusively, or use oneOf if plaintext support must remain while preventing both fields from being supplied.
🤖 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 `@openapi/ga/individual/platform.openapi.yaml`:
- Around line 19085-19116: Update the WorkloadTokenExchangeResponse schema by
removing scope from its required property list while keeping the scope property
definition and all other required fields unchanged.
In `@packages/nemo_platform_ext/tests/auth/test_workload_exchange.py`:
- Around line 145-174: Update
test_provider_rereads_subject_token_file_when_access_token_expires so the
initial get_access_token() call expects the already-expired exchanged token to
be rejected, matching _exchange validation. Preserve the subsequent refresh flow
and assertions by invoking the provider again after handling the expected
rejection, then verify it rereads subject-token-two and returns
fresh_access_token.
In `@packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py`:
- Around line 566-580: Update OIDC client method _exchange to validate the token
response’s expiry before assigning the result to self.tokens. Reject or avoid
caching an already-expired access token, matching the existing handling in the
sibling workload_exchange implementation, while preserving normal caching for
valid tokens.
---
Duplicate comments:
In `@contrib/auth/authentik/helm/templates/shared-postgres-initdb-configmap.yaml`:
- Around line 18-27: Update the initdb psql invocation to stop passing
authentik_password and nemo_password through --set command-line arguments;
provide both secrets through the heredoc/stdin using psql variable handling, and
preserve their use in the CREATE USER statements while keeping passwords out of
process argv.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 47-69: Update token_exchange_grant to validate token_endpoint with
the existing _validate_token_endpoint helper before calling httpx.post,
rejecting non-HTTPS or otherwise invalid schemes while preserving the current
token exchange request behavior.
- Around line 146-161: The _exchange method caches exchanged tokens without
validating their expiration. After constructing the TokenSet, verify it is still
valid using the existing token-expiration mechanism; reject or avoid caching an
already-expired token so get_access_token does not return it.
In `@packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py`:
- Around line 215-227: Restrict the HTTP acceptance logic in
_validate_token_endpoint to loopback hostnames only, regardless of allow_http.
Remove the allow_http branch that permits arbitrary HTTP hosts, while preserving
HTTPS acceptance and the existing localhost, 127.0.0.1, and ::1 behavior.
In `@packages/nemo_platform_plugin/tests/test_client_auth.py`:
- Around line 298-309: Update the token exchange tests around
test_token_exchange_grant_allows_http_when_platform_base_url_is_http and the
corresponding tests near the later covered range to expect rejection when
allow_http=True is used with non-loopback HTTP endpoints such as nemo-gateway.
Replace successful token assertions with validation of the appropriate error,
while preserving successful behavior for loopback HTTP endpoints.
In
`@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py`:
- Around line 56-68: Update WorkloadToken’s issue method to validate
self.token_endpoint before constructing or sending the credential-bearing
httpx.post request, rejecting non-HTTPS endpoints while allowing HTTP only for
explicitly permitted loopback development hosts. Preserve the existing token
request flow for valid endpoints and fail clearly before transmitting
credentials when validation fails.
---
Nitpick comments:
In `@contrib/auth/authentik/helm/templates/workload-token-tls.yaml`:
- Around line 14-19: Update the Secret metadata in the workload-token TLS
template to include the chart’s shared labels via the existing
nemo-authentik-demo.labels helper, while preserving the
app.kubernetes.io/component label and current metadata fields.
- Around line 10-11: Move the genCA and genSignedCert calls associated with $ca
and $cert inside the branch that creates new TLS material, after the
$hasExistingTls check. Ensure existing secrets reuse their certificate without
invoking either generation function, while preserving the current generation
inputs for new secrets.
In `@contrib/auth/manifest.schema.yaml`:
- Around line 68-94: The auth manifest schema permits plaintext password values
and does not enforce exclusivity. Update both human_grant and machine_grant to
remove the password property and require password_env_var exclusively, or use
oneOf if plaintext support must remain while preventing both fields from being
supplied.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 47-161: Replace the duplicated token exchange implementation in
token_exchange_grant and WorkloadTokenExchangeProvider with the existing shared
implementation from nemo_platform_plugin.client.oidc, or centralize it in a
common module consumed by both packages. Preserve the current public behavior
while ensuring security checks such as the plugin’s HTTPS validation remain
consistently applied.
🪄 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: 4709f201-c3a6-4c5e-acea-bd2e79aac1ae
⛔ Files ignored due to path filters (33)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/api.mdis excluded by!sdk/**sdk/python/nemo-platform/scripts/lintis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_config_map_volume.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_key_to_path.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_secret_volume.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_volume.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/workload_token_exchange_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (102)
.github/actions/setup-kind-cluster/action.yaml.github/workflows/ci.yamlMakefilecontrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/helm/.helmignorecontrib/auth/authentik/helm/Chart.yamlcontrib/auth/authentik/helm/files/blueprints/nemo.yamlcontrib/auth/authentik/helm/templates/_helpers.tplcontrib/auth/authentik/helm/templates/blueprint-apply-job.yamlcontrib/auth/authentik/helm/templates/blueprint-configmap.yamlcontrib/auth/authentik/helm/templates/shared-postgres-initdb-configmap.yamlcontrib/auth/authentik/helm/templates/shared-postgres-nemo-secret.yamlcontrib/auth/authentik/helm/templates/shared-postgres-secret.yamlcontrib/auth/authentik/helm/templates/shared-postgres-service.yamlcontrib/auth/authentik/helm/templates/shared-postgres-serviceaccount.yamlcontrib/auth/authentik/helm/templates/shared-postgres-statefulset.yamlcontrib/auth/authentik/helm/templates/tokenreview-rbac.yamlcontrib/auth/authentik/helm/templates/workload-token-signing-key-secret.yamlcontrib/auth/authentik/helm/templates/workload-token-tls.yamlcontrib/auth/authentik/helm/values.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyk8s/helm/README.mdk8s/helm/ci/21-api-extra-volumes.yamlk8s/helm/ci/22-envoy-config-override.yamlk8s/helm/templates/api/api-deployment.yamlk8s/helm/templates/platform-seed-job.yamlk8s/helm/templates/proxy/_helpers.tplk8s/helm/templates/proxy/envoy-configmap.yamlk8s/helm/templates/proxy/envoy-deployment.yamlk8s/helm/values.yamlopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/config.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nemo_platform_plugin/tests/test_config.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/config/__init__.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/test_discovery.pyservices/core/auth/tests/test_workload_token_exchange.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/core/jobs/tests/test_config.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/authentik_live.pytests/auth_idp/providers.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/vendor/vendor_package.pytools/nemo-platform-sdk-tools/tests/sdk/openapi_test.pytools/nemo-platform-sdk-tools/tests/sdk/vendor/test_vendor_package.py
💤 Files with no reviewable changes (4)
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- Makefile
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
🚧 Files skipped from review as they are similar to previous changes (66)
- contrib/auth/authentik/.gitignore
- contrib/auth/authentik/helm/.helmignore
- k8s/helm/templates/platform-seed-job.yaml
- contrib/auth/authentik/helm/templates/tokenreview-rbac.yaml
- contrib/auth/authentik/helm/templates/blueprint-configmap.yaml
- contrib/auth/authentik/helm/templates/shared-postgres-nemo-secret.yaml
- tests/auth_idp/authentik_live.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.py
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- tests/auth_idp/test_docs_links.py
- tests/auth_idp/test_fixture_helpers.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- k8s/helm/ci/22-envoy-config-override.yaml
- k8s/helm/ci/21-api-extra-volumes.yaml
- k8s/helm/README.md
- k8s/helm/templates/api/api-deployment.yaml
- docs/auth/deployment/configuration.mdx
- k8s/helm/templates/proxy/envoy-configmap.yaml
- e2e/services_pool.py
- packages/nmp_common/src/nmp/common/config/init.py
- services/core/auth/src/nmp/core/auth/service.py
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.py
- contrib/auth/authentik/helm/templates/_helpers.tpl
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- tests/auth_idp/test_authentik_blueprint.py
- docs/auth/authentication/idp-integration.mdx
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- contrib/auth/authentik/docker-compose.yml
- contrib/auth/authentik/manifest.yaml
- services/core/auth/tests/test_discovery.py
- docs/auth/deployment/credential-propagation.mdx
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- k8s/helm/templates/proxy/envoy-deployment.yaml
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- contrib/auth/authentik/blueprints/nemo.yaml
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- services/core/jobs/tests/controllers/test_workload_tokens.py
- contrib/auth/authentik/helm/files/blueprints/nemo.yaml
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- packages/nemo_platform_plugin/tests/test_config.py
- tests/auth_idp/test_provider_manifest.py
- k8s/helm/values.yaml
- tools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
- contrib/auth/authentik/README.md
- contrib/auth/authentik/gateway/envoy.yaml
- services/core/jobs/tests/test_config.py
- e2e/conftest.py
- docs/set-up/config-reference.mdx
- packages/nmp_common/src/nmp/common/auth/middleware.py
- packages/nemo_platform_ext/tests/config/test_config.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/config.py
- services/core/jobs/jobs-launcher/cmd/run.go
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- tests/auth_idp/test_provider_layout.py
- packages/nmp_common/src/nmp/common/config/base.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
- .github/workflows/ci.yaml
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- contrib/auth/authentik/run.sh
- tests/auth_idp/providers.py
- services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py
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 `@tests/auth_idp/test_authentik_kubernetes_demo.py`:
- Line 276: Update the assertion in the CI workflow test to match the complete
`${{ needs.build-cpu-smoke-images.outputs.publish_images }}` expression,
including its closing syntax, rather than only asserting a prefix. Preserve the
existing `NMP_AUTHENTIK_K8S_SKIP_IMAGE_LOAD` context.
🪄 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: 0a67bc03-6e75-4582-b49e-51c38bd7c1cc
⛔ Files ignored due to path filters (2)
sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**
📒 Files selected for processing (8)
.github/workflows/ci.yamlcontrib/auth/authentik/helm/templates/shared-postgres-initdb-configmap.yamlk8s/helm/ci/22-envoy-config-override.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/tests/test_client_auth.pytests/auth_idp/test_authentik_kubernetes_demo.py
🚧 Files skipped from review as they are similar to previous changes (7)
- contrib/auth/authentik/helm/templates/shared-postgres-initdb-configmap.yaml
- k8s/helm/ci/22-envoy-config-override.yaml
- packages/nemo_platform_ext/tests/auth/test_workload_exchange.py
- packages/nemo_platform_plugin/tests/test_client_auth.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py
- .github/workflows/ci.yaml
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py
Signed-off-by: Ryan S <267728323+ironcommit@users.noreply.github.com>
ironcommit
force-pushed
the
auth-idp-3/rsadler
branch
from
ccb94fe to
1b250da
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contrib/auth/authentik/run.sh (1)
399-464: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winScope and validate Kubernetes-only flag combinations.
--skip-image-loadis accepted without--reuse-cluster, creating a fresh-cluster mode that skips its image transfer. Also,NMP_AUTHENTIK_K8S_REUSE_CLUSTER=1makes unrelated actions fail at Line 462. Require reuse for skip-image-load and run this validation only fork8s-test.Proposed fix
-if [[ "${K8S_REUSE_CLUSTER}" == "1" && -z "${K8S_CLUSTER_NAME}" ]]; then - die "--reuse-cluster requires --cluster-name or NMP_AUTHENTIK_K8S_CLUSTER_NAME" +if [[ "${ACTION}" == "k8s-test" ]]; then + if [[ "${K8S_SKIP_IMAGE_LOAD}" == "1" && "${K8S_REUSE_CLUSTER}" != "1" ]]; then + die "--skip-image-load requires --reuse-cluster" + fi + if [[ "${K8S_REUSE_CLUSTER}" == "1" && -z "${K8S_CLUSTER_NAME}" ]]; then + die "--reuse-cluster requires --cluster-name or NMP_AUTHENTIK_K8S_CLUSTER_NAME" + fi fi🤖 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 `@contrib/auth/authentik/run.sh` around lines 399 - 464, Update the validation around K8S_REUSE_CLUSTER and K8S_SKIP_IMAGE_LOAD so it runs only when ACTION is "k8s-test"; within that scope, require --reuse-cluster whenever --skip-image-load is set, while preserving the existing cluster-name requirement for reuse. Prevent NMP_AUTHENTIK_K8S_REUSE_CLUSTER from rejecting unrelated actions.
♻️ Duplicate comments (2)
packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py (1)
69-69: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject non-HTTPS token endpoints.
subject_tokenis posted without validatingtoken_endpoint; anhttp://endpoint exposes the workload credential in cleartext. Reuse the existing OIDC endpoint validation before this request.🤖 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/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py` at line 69, Validate token_endpoint with the existing OIDC endpoint validation before the httpx.post call in the workload exchange flow. Reject non-HTTPS endpoints before posting subject_token, while preserving the current request behavior for valid endpoints.Source: Linters/SAST tools
packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py (1)
215-227: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
allow_httplets workload token exchange leak subject tokens in cleartext to any remote host. Root cause:_validate_token_endpointtreatsallow_http=Trueas "accept anyhttp://scheme" instead of restricting to loopback hosts;allow_httpis derived from the platform's own base URL, not the actual token endpoint's host.
packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py#L215-L227: restrict theallow_httpbranch to loopback/local hostnames (e.g.localhost,127.0.0.1,::1) instead of anyhttpscheme.packages/nemo_platform_plugin/tests/test_client_auth.py#L298-L310: once fixed, updatetest_token_exchange_grant_allows_http_when_platform_base_url_is_httpso a non-loopbackhttp://endpoint is rejected even withallow_http=True.🤖 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/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py` around lines 215 - 227, Restrict the allow_http branch in _validate_token_endpoint to HTTP endpoints whose hostname is loopback/local (localhost, 127.0.0.1, or ::1), preventing remote cleartext endpoints; retain HTTPS acceptance and the existing default loopback behavior. In packages/nemo_platform_plugin/tests/test_client_auth.py lines 298-310, update test_token_exchange_grant_allows_http_when_platform_base_url_is_http to expect rejection for a non-loopback HTTP endpoint even when allow_http=True.
🧹 Nitpick comments (1)
contrib/auth/authentik/helm/templates/workload-token-tls.yaml (1)
9-29: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueDefer cert/CA generation until actually needed.
genCA/genSignedCertrun unconditionally even when$hasExistingTlsis true and their output is discarded.♻️ Move generation into the else branch
-{{- $hasExistingTls := and (hasKey $existingData "tls.crt") (hasKey $existingData "tls.key") (hasKey $existingData "ca.crt") -}} -{{- $ca := genCA (printf "%s-ca" $secretName) $durationDays -}} -{{- $cert := genSignedCert $secretName .Values.workloadTokenTls.ipAddresses .Values.workloadTokenTls.dnsNames $durationDays $ca -}} +{{- $hasExistingTls := and (hasKey $existingData "tls.crt") (hasKey $existingData "tls.key") (hasKey $existingData "ca.crt") -}} +{{- if not $hasExistingTls -}} +{{- $ca := genCA (printf "%s-ca" $secretName) $durationDays -}} +{{- $cert := genSignedCert $secretName .Values.workloadTokenTls.ipAddresses .Values.workloadTokenTls.dnsNames $durationDays $ca -}} +{{- end -}} apiVersion: v1 kind: Secret metadata: name: {{ $secretName }} namespace: {{ include "nemo-authentik-demo.namespace" . }} labels: app.kubernetes.io/component: workload-token-tls type: kubernetes.io/tls data: {{- if $hasExistingTls }} tls.crt: {{ index $existingData "tls.crt" | quote }} tls.key: {{ index $existingData "tls.key" | quote }} ca.crt: {{ index $existingData "ca.crt" | quote }} {{- else }} tls.crt: {{ $cert.Cert | b64enc | quote }} tls.key: {{ $cert.Key | b64enc | quote }} ca.crt: {{ $ca.Cert | b64enc | quote }} {{- end }}🤖 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 `@contrib/auth/authentik/helm/templates/workload-token-tls.yaml` around lines 9 - 29, Move the $ca and $cert generation assignments into the else branch guarded by $hasExistingTls in the workload-token TLS Secret template. Keep existing TLS data handling unchanged, and generate certificates only when no complete existing TLS data is available.
🤖 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.
Outside diff comments:
In `@contrib/auth/authentik/run.sh`:
- Around line 399-464: Update the validation around K8S_REUSE_CLUSTER and
K8S_SKIP_IMAGE_LOAD so it runs only when ACTION is "k8s-test"; within that
scope, require --reuse-cluster whenever --skip-image-load is set, while
preserving the existing cluster-name requirement for reuse. Prevent
NMP_AUTHENTIK_K8S_REUSE_CLUSTER from rejecting unrelated actions.
---
Duplicate comments:
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Line 69: Validate token_endpoint with the existing OIDC endpoint validation
before the httpx.post call in the workload exchange flow. Reject non-HTTPS
endpoints before posting subject_token, while preserving the current request
behavior for valid endpoints.
In `@packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py`:
- Around line 215-227: Restrict the allow_http branch in
_validate_token_endpoint to HTTP endpoints whose hostname is loopback/local
(localhost, 127.0.0.1, or ::1), preventing remote cleartext endpoints; retain
HTTPS acceptance and the existing default loopback behavior. In
packages/nemo_platform_plugin/tests/test_client_auth.py lines 298-310, update
test_token_exchange_grant_allows_http_when_platform_base_url_is_http to expect
rejection for a non-loopback HTTP endpoint even when allow_http=True.
---
Nitpick comments:
In `@contrib/auth/authentik/helm/templates/workload-token-tls.yaml`:
- Around line 9-29: Move the $ca and $cert generation assignments into the else
branch guarded by $hasExistingTls in the workload-token TLS Secret template.
Keep existing TLS data handling unchanged, and generate certificates only when
no complete existing TLS data is available.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a9aa4c51-2822-4cfd-ba94-6192f605f285
⛔ Files ignored due to path filters (33)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/api.mdis excluded by!sdk/**sdk/python/nemo-platform/scripts/lintis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_config_map_volume.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_key_to_path.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_secret_volume.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_volume.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/workload_token_exchange_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (102)
.github/actions/setup-kind-cluster/action.yaml.github/workflows/ci.yamlMakefilecontrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/helm/.helmignorecontrib/auth/authentik/helm/Chart.yamlcontrib/auth/authentik/helm/files/blueprints/nemo.yamlcontrib/auth/authentik/helm/templates/_helpers.tplcontrib/auth/authentik/helm/templates/blueprint-apply-job.yamlcontrib/auth/authentik/helm/templates/blueprint-configmap.yamlcontrib/auth/authentik/helm/templates/shared-postgres-initdb-configmap.yamlcontrib/auth/authentik/helm/templates/shared-postgres-nemo-secret.yamlcontrib/auth/authentik/helm/templates/shared-postgres-secret.yamlcontrib/auth/authentik/helm/templates/shared-postgres-service.yamlcontrib/auth/authentik/helm/templates/shared-postgres-serviceaccount.yamlcontrib/auth/authentik/helm/templates/shared-postgres-statefulset.yamlcontrib/auth/authentik/helm/templates/tokenreview-rbac.yamlcontrib/auth/authentik/helm/templates/workload-token-signing-key-secret.yamlcontrib/auth/authentik/helm/templates/workload-token-tls.yamlcontrib/auth/authentik/helm/values.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyk8s/helm/README.mdk8s/helm/ci/21-api-extra-volumes.yamlk8s/helm/ci/22-envoy-config-override.yamlk8s/helm/templates/api/api-deployment.yamlk8s/helm/templates/platform-seed-job.yamlk8s/helm/templates/proxy/_helpers.tplk8s/helm/templates/proxy/envoy-configmap.yamlk8s/helm/templates/proxy/envoy-deployment.yamlk8s/helm/values.yamlopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/config.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nemo_platform_plugin/tests/test_config.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/config/__init__.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/test_discovery.pyservices/core/auth/tests/test_workload_token_exchange.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/core/jobs/tests/test_config.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/authentik_live.pytests/auth_idp/providers.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/vendor/vendor_package.pytools/nemo-platform-sdk-tools/tests/sdk/openapi_test.pytools/nemo-platform-sdk-tools/tests/sdk/vendor/test_vendor_package.py
💤 Files with no reviewable changes (4)
- Makefile
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
🚧 Files skipped from review as they are similar to previous changes (68)
- contrib/auth/authentik/helm/.helmignore
- contrib/auth/authentik/helm/Chart.yaml
- docs/auth/authentication/idp-integration.mdx
- packages/nmp_common/src/nmp/common/config/init.py
- tests/auth_idp/authentik_live.py
- contrib/auth/authentik/.gitignore
- tests/auth_idp/test_docs_links.py
- k8s/helm/ci/21-api-extra-volumes.yaml
- contrib/auth/authentik/helm/templates/blueprint-configmap.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- contrib/auth/authentik/helm/templates/tokenreview-rbac.yaml
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.py
- k8s/helm/templates/api/api-deployment.yaml
- tools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
- e2e/services_pool.py
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- packages/nmp_common/src/nmp/common/auth/middleware.py
- .github/actions/setup-kind-cluster/action.yaml
- k8s/helm/values.yaml
- services/core/auth/src/nmp/core/auth/service.py
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/vendor/vendor_package.py
- docs/auth/deployment/credential-propagation.mdx
- contrib/auth/authentik/blueprints/nemo.yaml
- contrib/auth/authentik/gateway/envoy.yaml
- k8s/helm/templates/platform-seed-job.yaml
- k8s/helm/templates/proxy/_helpers.tpl
- k8s/helm/README.md
- k8s/helm/templates/proxy/envoy-deployment.yaml
- services/core/auth/tests/test_discovery.py
- docs/auth/deployment/configuration.mdx
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- tools/nemo-platform-sdk-tools/tests/sdk/vendor/test_vendor_package.py
- k8s/helm/ci/22-envoy-config-override.yaml
- packages/nemo_platform_plugin/tests/test_config.py
- contrib/auth/authentik/docker-compose.yml
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- contrib/auth/authentik/helm/files/blueprints/nemo.yaml
- tests/auth_idp/test_authentik_blueprint.py
- packages/nemo_platform_ext/tests/config/test_config.py
- contrib/auth/authentik/helm/templates/_helpers.tpl
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- services/core/jobs/tests/test_config.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.py
- packages/nmp_common/src/nmp/common/config/base.py
- tests/auth_idp/test_provider_manifest.py
- services/core/jobs/jobs-launcher/cmd/run.go
- tests/auth_idp/providers.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/config.py
- e2e/conftest.py
- docs/set-up/config-reference.mdx
- tests/auth_idp/test_fixture_helpers.py
- contrib/auth/authentik/helm/values.yaml
- contrib/auth/authentik/README.md
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- tests/auth_idp/test_provider_layout.py
- services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- services/core/jobs/tests/controllers/test_workload_tokens.py
- .github/workflows/ci.yaml
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
- contrib/auth/manifest.schema.yaml
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
4 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
Replace static workload token injection (
NEMO_WORKLOAD_TOKEN/NEMO_WORKLOAD_TOKEN_FILE) with managed, file-based workload identity token exchange following RFC 8693. Backends now own the subject token lifecycle — writing and refreshing a token file — while the SDK performs just-in-time exchange, mirroring the pattern used by AWS, Azure, and GCP.Changes
Authentik IdP Reference Demo
nemo-platform-workloadOAuth2 provider and application with separate, shorter token lifetimes for workload identityworkload_exchange_broker.py) — a FastAPI service implementing RFC 8693 token exchange, since Authentik 2024.12 does not support the grant nativelynemo.yaml.tpl) with configurable token lifetime knobs;run.sh render-blueprintrenders it with environment-driven defaultsrequires_any)docker-compose.ymlto include the broker service and expose the platform on aworkloadnetworkKubernetes Reference Demo (new)
contrib/auth/authentik/kubernetes/— Authentik server/worker, Postgres, Redis, Envoy gateway, and the workload exchange brokerrun.sh k8s-rendertests/auth_idp_k8s/) gated behindNMP_AUTHENTIK_K8S_E2E=1Job Backends
/var/run/secrets/nemo-platform/workload, and injectNMP_WORKLOAD_IDENTITY_TOKEN_FILEinto container envvalidate_no_reserved_managed_job_environment_variable_namesat the backend layer to prevent user overrides of managed auth variablesconfigureOTELHeadersFromWorkloadToken()from jobs-launcher (no longer needed)SDK / Client
WorkloadTokenExchangeProvider— a thread-safe provider that reads the subject token file, performs RFC 8693 exchange, and caches the result until expiryNMP_WORKLOAD_IDENTITY_TOKEN_FILEand creates the exchange provider automatically (discovery-driven, no static config needed)NMP_ACCESS_TOKENtakes precedence; legacyNEMO_WORKLOAD_TOKEN/NEMO_WORKLOAD_TOKEN_FILEare now no-opsnemo_platform_extandnemo_platform_pluginpackagesAuth Discovery API
OIDCDiscoveryResponsewith workload exchange fields (workload_token_exchange_enabled,workload_client_id,workload_token_endpoint, etc.) populated from platform configPlatform Config
workload_*fields toOIDCConfiginnmp_commonNEMO_WORKLOAD_TOKEN/NEMO_WORKLOAD_TOKEN_FILEfrom config-layer token sourcesHello-World Workload Task
workload_workspace_gettask toNeMoPlatform()with no arguments — the SDK bootstrap handles token exchange automaticallyDocumentation
Testing
WorkloadTokenExchangeProvider, client factory workload identity path, config no-op behavior for legacy env vars, Docker/Kubernetes backend token handlingNMP_AUTHENTIK_K8S_E2E=1) that provisions a cluster, deploys the full stack, and validates token exchange end-to-endSummary by CodeRabbit
/tokenendpoints and expanded OIDC discovery fields.NMP_WORKLOAD_IDENTITY_TOKEN_FILE.