Skip to content

feat(auth): add access key listing and revocation - #1095

Merged
anastasia-nesterenko merged 8 commits into
mainfrom
anesterenko/aircore-983-implement-access-keys-revoke
Aug 13, 2026
Merged

feat(auth): add access key listing and revocation#1095
anastasia-nesterenko merged 8 commits into
mainfrom
anesterenko/aircore-983-implement-access-keys-revoke

Conversation

@anastasia-nesterenko

@anastasia-nesterenko anastasia-nesterenko commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds persistent lifecycle management for Scoped Access Keys across the auth service, Python SDK, and NeMo CLI. Previously keys could be created but not inventoried or revoked; this change stores lifecycle metadata, exposes list and revoke operations, and rejects expired or revoked keys during authentication.

Changes

  • Persist access-key metadata in the entities service without storing token values, scoped to the owning principal.
  • Report lifecycle status as ACTIVE, EXPIRED, or REVOKED, including legacy-key backfill and fail-closed validation for current keys.
  • Add paginated GET /apis/auth/v2/access-keys and idempotent DELETE /apis/auth/v2/access-keys/{jti} endpoints.
  • Add nemo auth access-keys list and nemo auth access-keys revoke <jti> commands, optional key descriptions, truncation warnings, and actionable error handling.
  • Enforce lifecycle state through the auth-service validation callout, with per-worker soft circuit breaking that continues to fail closed.
  • Normalize access-key audience metadata, prevent conflicting workload/access-key signing key IDs, and clear inherited delegation headers for service-level entity access.
  • Return stable public disabled-feature errors without exposing exception details.
  • Regenerate the OpenAPI specifications and Python SDK, refresh vendored CLI code, and update CLI and authentication documentation.
  • Expand unit and integration coverage for persistence, ownership, listing, revocation, authentication, legacy migration, middleware failure handling, CLI behavior, and generated SDK surfaces.
  • Reuse a module-scoped IGW and Models service context across the mock-provider suite while keeping provider names unique and state-isolation coverage explicit.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with documentation updates
  • Documentation only
  • Contributor tooling or automation
  • CI, build, or test infrastructure

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Documentation updated for user-visible behavior
  • Documentation not applicable — justification:

Verification

  • Pull request title follows the repository's Conventional Commit format
  • Every commit includes an appropriate Signed-off-by: trailer
  • uv run pre-commit run -a passes, or any blocked checks are identified below
  • Targeted tests pass, or tests are marked not applicable above
  • No secrets, API keys, or credentials are included

Targeted validation:

  • uv run pre-commit run -a — passed for the PR content.
  • uv run --frozen pytest packages/nemo_platform_ext/tests/cli/commands/test_auth.py packages/nemo_platform_ext/tests/cli/core/test_errors.py packages/nemo_platform_plugin/tests/auth/access_keys/test_client.py packages/nmp_common/tests/auth/test_access_keys.py packages/nmp_common/tests/auth/test_jwt.py packages/nmp_common/tests/auth/test_middleware.py services/core/auth/tests/test_access_key_registry.py services/core/auth/tests/test_access_keys.py -q — 259 passed.
  • uv run --frozen pytest tests/vendored/nemo_platform_ext/cli/commands/test_auth.py tests/vendored/nemo_platform_ext/cli/core/test_errors.py -q from sdk/python/nemo-platform — 90 passed.
  • uv run --frozen pytest services/core/auth/tests/integration/test_scoped_access_keys.py -v — 1 passed.
  • uv run --frozen pytest -n 4 --maxprocesses=16 --max-worker-restart=0 --dist loadgroup --timeout=120 -vv services/core/inference-gateway/tests/integration/test_igw_with_auth.py services/core/inference-gateway/tests/integration/test_middleware_pipeline.py — 37 passed.
  • UV_PYTHON=3.12 uv run --frozen pytest -n 3 --maxprocesses=16 --max-worker-restart=0 --dist loadgroup --timeout=120 -q services/core/inference-gateway/tests/integration/test_mock_provider_mode.py — 54 passed.
  • uv run --frozen ruff check services/core/inference-gateway/tests/integration/test_middleware_pipeline.py services/core/inference-gateway/tests/integration/test_igw_with_auth.py — passed.
  • uv run --frozen ruff format --check services/core/inference-gateway/tests/integration/test_middleware_pipeline.py services/core/inference-gateway/tests/integration/test_igw_with_auth.py — passed.

Summary by CodeRabbit

  • New Features
    • Scoped Access Keys can now be listed with pagination and revoked through the CLI and API.
    • Access-key creation supports optional descriptions.
    • Access-key responses include lifecycle status, issuer, audiences, expiration, and revocation results.
    • Authentication rejects revoked or inactive access keys.
  • Bug Fixes
    • Improved handling of disabled features, missing keys, malformed identifiers, timeouts, and API errors.
  • Documentation
    • Expanded guidance for key lifecycle management, metadata, pagination, revocation, and current limitations.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 32405/41076 78.9% 63.8%
Integration Tests 18797/38978 48.2% 21.0%

@anastasia-nesterenko
anastasia-nesterenko force-pushed the anesterenko/aircore-983-implement-access-keys-revoke branch from d0efd70 to 93a8189 Compare August 5, 2026 16:59
@anastasia-nesterenko anastasia-nesterenko changed the title Add access keys revoke and list feat(auth): add access key listing and revocation Aug 5, 2026
@github-actions github-actions Bot added the feat label Aug 5, 2026
@anastasia-nesterenko
anastasia-nesterenko force-pushed the anesterenko/aircore-983-implement-access-keys-revoke branch 7 times, most recently from 1e539f7 to a8d7976 Compare August 7, 2026 17:57
@anastasia-nesterenko
anastasia-nesterenko force-pushed the anesterenko/aircore-983-implement-access-keys-revoke branch from a8d7976 to 6ae7a2d Compare August 7, 2026 18:59
@anastasia-nesterenko
anastasia-nesterenko marked this pull request as ready for review August 7, 2026 21:51
@anastasia-nesterenko
anastasia-nesterenko requested review from a team as code owners August 7, 2026 21:51
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔄 Running review...
📝 Walkthrough

Walkthrough

Scoped Access Keys now support persisted lifecycle metadata, paginated listing, revocation, structured API responses, CLI lifecycle commands, and lifecycle-aware bearer authentication. Tests and documentation cover the new behavior.

Changes

Scoped Access Key lifecycle

Layer / File(s) Summary
API contracts and client types
openapi/..., packages/nemo_platform_plugin/src/.../access_keys/*
The API and plugin types define descriptions, statuses, pagination, structured errors, JTI validation, and revocation responses.
Persistence and API operations
services/core/auth/src/.../access_keys/*, services/core/auth/src/.../entities/*
Access-key metadata is persisted. Listing is principal-scoped and paginated. Revocation records lifecycle state and handles conflicts.
Lifecycle authentication
packages/nmp_common/src/.../auth/*, services/core/auth/src/.../authenticate.py
Access-key candidates use lifecycle validation. Registry activity, JWT claims, unavailable-service responses, and circuit breaking are handled.
CLI and client integration
packages/nemo_platform_ext/src/.../auth.py, packages/nemo_platform_plugin/src/.../access_keys/*
CLI creation accepts descriptions. Listing supports pagination and formatted output. Revocation reports newly revoked and already revoked states.
Documentation and supporting wiring
docs/auth/*, docs/cli/reference.mdx, packages/nmp_platform_runner/src/*, packages/nmp_testing/src/*
Documentation describes lifecycle operations. Separate lifecycle HTTP-client wiring is passed through application setup and test clients.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant AuthAPI
  participant PersistentAccessKeyIssuer
  participant AccessKeyRegistry
  participant AuthorizationMiddleware
  Client->>AuthAPI: Create, list, or revoke access key
  AuthAPI->>PersistentAccessKeyIssuer: Execute lifecycle operation
  PersistentAccessKeyIssuer->>AccessKeyRegistry: Persist or update lifecycle metadata
  AccessKeyRegistry-->>AuthAPI: Return metadata or revocation result
  Client->>AuthorizationMiddleware: Send access-key bearer token
  AuthorizationMiddleware->>AuthAPI: Validate lifecycle state
  AuthAPI->>AccessKeyRegistry: Check active JTI for principal
  AccessKeyRegistry-->>AuthorizationMiddleware: Return active claims or rejection
Loading

Suggested reviewers: a2bondar

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.94% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies access key listing and revocation, which are central changes in the pull request.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch anesterenko/aircore-983-implement-access-keys-revoke

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
openapi/ga/individual/platform.openapi.yaml (1)

260-284: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Accept legacy JTIs in the revoke path.

Backfill persists the original legacy jti without normalization. Values such as ak_legacy do not match ^ak_[0-9a-f]{32}$, so revoke requests receive 422 before the handler runs. Remove the pattern or broaden it, and add a backfilled-key revoke test.

🤖 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 260 - 284, Update
the revoke_access_key path parameter schema for jti to accept legacy persisted
JTIs such as ak_legacy by removing or broadening the restrictive hexadecimal
pattern, while keeping the parameter required and string-typed. Add a test
covering revocation of a backfilled key with a legacy JTI and verify the handler
succeeds instead of returning validation error 422.
openapi/ga/openapi.yaml (1)

267-275: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Allow legacy jti values in the revoke path.

New keys use the required format, but backfill stores legacy values unchanged. The ak_legacy fixture does not match ^ak_[0-9a-f]{32}$; DELETE therefore returns 422 instead of revoking the key. Accept legacy values or migrate them before enforcing this pattern.

🤖 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 267 - 275, Update the revoke-path jti
parameter schema to accept legacy stored values such as ak_legacy while
retaining validation for the new ak_ plus 32-hex-character format. Remove or
broaden the restrictive pattern on the jti path parameter so DELETE revocation
does not return 422 for backfilled legacy keys.
🧹 Nitpick comments (2)
packages/nmp_common/src/nmp/common/auth/access_keys.py (1)

402-414: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Downgrade the catch-all log to debug or throttle it.

Any invalid or tampered access-key token now emits a warning with a stack trace. An unauthenticated caller can flood logs by sending garbage bearer tokens.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/nmp_common/src/nmp/common/auth/access_keys.py` around lines 402 -
414, Update the broad exception handler around TokenClaims construction to log
validation failures at debug level instead of warning, and disable stack-trace
logging for these expected invalid-token cases. Preserve the existing exception
propagation for httpx.HTTPError.
packages/nmp_common/tests/auth/test_middleware.py (1)

826-833: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Close the httpx.AsyncClient.

This client is created but never closed, so the test can emit unclosed-client warnings. Other tests here use async with. Wrap it or register a cleanup.

🤖 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/tests/auth/test_middleware.py` around lines 826 - 833,
Update the test setup around the AsyncClient used by AuthorizationMiddleware to
ensure the httpx.AsyncClient is closed after the test completes. Prefer an
async-context-manager or the test’s existing cleanup mechanism, while keeping
the same client instance supplied to both middleware parameters.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/nmp_common/src/nmp/common/auth/access_keys.py`:
- Around line 92-107: Update is_access_key_token_candidate to catch the broader
jwt.PyJWTError base exception around jwt.decode, returning False for any
malformed or otherwise invalid token while preserving the existing token-type
check for successfully decoded payloads.

In `@packages/nmp_platform_runner/src/nmp/platform_runner/server.py`:
- Around line 235-240: Update create_app and its AuthorizationMiddleware setup
to use a dedicated lifecycle HTTP client for lifecycle requests instead of
reusing the endpoint-bound http_client. Add and thread a separate
lifecycle-client parameter through the relevant call sites, or explicitly
enforce and document that the injected client must reach the auth endpoint.

---

Outside diff comments:
In `@openapi/ga/individual/platform.openapi.yaml`:
- Around line 260-284: Update the revoke_access_key path parameter schema for
jti to accept legacy persisted JTIs such as ak_legacy by removing or broadening
the restrictive hexadecimal pattern, while keeping the parameter required and
string-typed. Add a test covering revocation of a backfilled key with a legacy
JTI and verify the handler succeeds instead of returning validation error 422.

In `@openapi/ga/openapi.yaml`:
- Around line 267-275: Update the revoke-path jti parameter schema to accept
legacy stored values such as ak_legacy while retaining validation for the new
ak_ plus 32-hex-character format. Remove or broaden the restrictive pattern on
the jti path parameter so DELETE revocation does not return 422 for backfilled
legacy keys.

---

Nitpick comments:
In `@packages/nmp_common/src/nmp/common/auth/access_keys.py`:
- Around line 402-414: Update the broad exception handler around TokenClaims
construction to log validation failures at debug level instead of warning, and
disable stack-trace logging for these expected invalid-token cases. Preserve the
existing exception propagation for httpx.HTTPError.

In `@packages/nmp_common/tests/auth/test_middleware.py`:
- Around line 826-833: Update the test setup around the AsyncClient used by
AuthorizationMiddleware to ensure the httpx.AsyncClient is closed after the test
completes. Prefer an async-context-manager or the test’s existing cleanup
mechanism, while keeping the same client instance supplied to both middleware
parameters.
🪄 Autofix

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: 8ef44860-452c-4f37-bf43-151225a4dd59

📥 Commits

Reviewing files that changed from the base of the PR and between c0dbe85 and 1afcd3b.

⛔ Files ignored due to path filters (17)
  • sdk/python/nemo-platform/.nmpcontext/openapi.yaml is excluded by !sdk/**
  • sdk/python/nemo-platform/.nmpcontext/stainless.yaml is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/cli/core/errors.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/access_keys/access_keys.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/access_keys/api.md is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/__init__.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/access_key_create_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/access_key_create_response.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/access_key_list_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/access_key_list_response.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/access_key_metadata_response.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/access_key_revoke_response.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/test_access_keys.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/core/test_errors.py is excluded by !sdk/**
  • sdk/stainless.yaml is excluded by !sdk/**
📒 Files selected for processing (41)
  • docs/auth/authentication/using-authentication.mdx
  • docs/auth/deployment/configuration.mdx
  • docs/cli/reference.mdx
  • openapi/ga/individual/platform.openapi.yaml
  • openapi/ga/openapi.yaml
  • openapi/openapi.yaml
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/core/errors.py
  • packages/nemo_platform_ext/tests/cli/commands/test_auth.py
  • packages/nemo_platform_ext/tests/cli/core/test_errors.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/access_keys/client.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/access_keys/endpoints.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/access_keys/issuer.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/access_keys/types.py
  • packages/nemo_platform_plugin/tests/auth/access_keys/test_client.py
  • packages/nemo_platform_plugin/tests/auth/access_keys/test_endpoints.py
  • packages/nmp_common/src/nmp/common/auth/access_keys.py
  • packages/nmp_common/src/nmp/common/auth/jwt.py
  • packages/nmp_common/src/nmp/common/auth/middleware.py
  • packages/nmp_common/src/nmp/common/config/base.py
  • packages/nmp_common/src/nmp/common/entities/client.py
  • packages/nmp_common/tests/auth/test_access_keys.py
  • packages/nmp_common/tests/auth/test_jwt.py
  • packages/nmp_common/tests/auth/test_middleware.py
  • packages/nmp_common/tests/entities/test_client.py
  • packages/nmp_platform_runner/src/nmp/platform_runner/server.py
  • packages/nmp_platform_runner/tests/test_server.py
  • services/core/auth/src/nmp/core/auth/api/v2/access_keys/endpoints.py
  • services/core/auth/src/nmp/core/auth/api/v2/access_keys/schemas.py
  • services/core/auth/src/nmp/core/auth/api/v2/authenticate.py
  • services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py
  • services/core/auth/src/nmp/core/auth/app/access_keys.py
  • services/core/auth/src/nmp/core/auth/entities/__init__.py
  • services/core/auth/src/nmp/core/auth/entities/entities.py
  • services/core/auth/tests/integration/test_scoped_access_keys.py
  • services/core/auth/tests/test_access_key_registry.py
  • services/core/auth/tests/test_access_keys.py
  • services/core/auth/tests/test_authenticate.py
  • services/core/auth/tests/test_embedded_pdp_stress.py
  • services/core/auth/tests/test_workload_token_exchange.py
  • services/core/inference-gateway/tests/integration/test_middleware_pipeline.py

Comment thread packages/nmp_common/src/nmp/common/auth/access_keys.py Outdated
Comment thread packages/nmp_platform_runner/src/nmp/platform_runner/server.py
Comment thread packages/nemo_platform_ext/src/nemo_platform_ext/cli/core/errors.py
Comment thread packages/nmp_common/src/nmp/common/auth/middleware.py Outdated
Comment thread packages/nmp_common/tests/auth/test_access_keys.py Outdated
Comment thread services/core/auth/src/nmp/core/auth/entities/entities.py Outdated
Comment thread services/core/auth/tests/test_access_keys.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@services/core/inference-gateway/tests/integration/test_mock_provider_mode.py`:
- Line 70: Rename the provider created by
test_fixture_add_provider_with_error_status so it uses a unique name instead of
duplicating the error-provider created by test_example_simulate_server_error.
Update all references within that fixture and its test flow consistently.
🪄 Autofix

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: f0b91852-db36-4df6-a314-a9271567e780

📥 Commits

Reviewing files that changed from the base of the PR and between 06cc0bb and c396666.

📒 Files selected for processing (2)
  • services/core/inference-gateway/tests/integration/test_igw_with_auth.py
  • services/core/inference-gateway/tests/integration/test_mock_provider_mode.py

@anastasia-nesterenko
anastasia-nesterenko requested a review from a team as a code owner August 8, 2026 00:01
@anastasia-nesterenko
anastasia-nesterenko force-pushed the anesterenko/aircore-983-implement-access-keys-revoke branch 2 times, most recently from 6dcc562 to 9c3ecd7 Compare August 11, 2026 17:18
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
packages/nmp_platform_runner/tests/test_server.py (1)

283-288: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the lifecycle client in the build tests.

Both fake_create_app functions accept access_key_lifecycle_http_client but discard it. The tests can pass even if build_platform_app stops forwarding the argument. Store the value, pass a non-None client to at least one call, and assert object identity.

Suggested test update
         captured["http_client"] = http_client
+        captured["access_key_lifecycle_http_client"] = access_key_lifecycle_http_client

     app = server.build_platform_app(
         ...
+        access_key_lifecycle_http_client=lifecycle_http_client,
     )
     ...
+    assert captured["access_key_lifecycle_http_client"] is lifecycle_http_client

Apply the capture assignment to both fakes.

Also applies to: 313-318

🤖 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_platform_runner/tests/test_server.py` around lines 283 - 288,
Update both fake_create_app functions to capture the
access_key_lifecycle_http_client argument, then pass a non-None client through
at least one build_platform_app test invocation and assert the captured value is
the identical object provided. Preserve the existing test setup while ensuring
both fakes record the lifecycle client.
packages/nmp_common/tests/auth/test_access_key_lifecycle.py (1)

81-107: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add circuit-breaker coverage.

The tests exercise the happy path, the non-access-key rejection, and one malformed response. The fail-closed circuit breaker is untested: no test drives three consecutive failures to open the circuit, asserts the retry_after value, asserts the 503 raised while the circuit is open, or covers the half-open reset after the window lapses. The AuthenticationErrorNone path, the missing-jti → 503 path, and the APITimeoutError → 504 path are also uncovered.

This is the logic that decides whether requests are accepted when the auth service degrades. Add cases that call authenticate repeatedly against a failing handler and assert status_code, retry_after, and recovery after the open window.

Run tests with uv run pytest packages/nmp_common/tests/auth/test_access_key_lifecycle.py.

I can draft these tests. Do you want me to?

As per coding guidelines: "Run Python scripts and tools through uv, such as uv run script.py, uv run pytest".

🤖 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/tests/auth/test_access_key_lifecycle.py` around lines 81
- 107, Expand test coverage around AccessKeyLifecycleAuthenticator.authenticate
for AuthenticationError returning None, missing jti producing 503, and
APITimeoutError producing 504. Add circuit-breaker tests that trigger three
consecutive failures, assert the open-circuit 503 and retry_after, verify
requests are blocked during the window, and confirm half-open recovery resets
the breaker after the window expires. Run the targeted file with uv run pytest
packages/nmp_common/tests/auth/test_access_key_lifecycle.py.

Source: Coding guidelines

packages/nmp_common/tests/auth/test_middleware.py (1)

850-894: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Close the httpx.AsyncClient created at Line 854.

The other lifecycle tests use async with. This test leaks the client, which produces unclosed-transport warnings. Close it after the request, for example with client.portal-free teardown via asyncio.run(http_client.aclose()) in a finally block, or convert the test to async and use async with.

🤖 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/tests/auth/test_middleware.py` around lines 850 - 894,
Close the httpx.AsyncClient created in this test after the request completes.
Update the test lifecycle around the visible http_client setup and client.get
call to guarantee http_client.aclose() runs in a finally block, or convert the
test to async and manage it with async with, while preserving the existing
assertions.
🤖 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.

Nitpick comments:
In `@packages/nmp_common/tests/auth/test_access_key_lifecycle.py`:
- Around line 81-107: Expand test coverage around
AccessKeyLifecycleAuthenticator.authenticate for AuthenticationError returning
None, missing jti producing 503, and APITimeoutError producing 504. Add
circuit-breaker tests that trigger three consecutive failures, assert the
open-circuit 503 and retry_after, verify requests are blocked during the window,
and confirm half-open recovery resets the breaker after the window expires. Run
the targeted file with uv run pytest
packages/nmp_common/tests/auth/test_access_key_lifecycle.py.

In `@packages/nmp_common/tests/auth/test_middleware.py`:
- Around line 850-894: Close the httpx.AsyncClient created in this test after
the request completes. Update the test lifecycle around the visible http_client
setup and client.get call to guarantee http_client.aclose() runs in a finally
block, or convert the test to async and manage it with async with, while
preserving the existing assertions.

In `@packages/nmp_platform_runner/tests/test_server.py`:
- Around line 283-288: Update both fake_create_app functions to capture the
access_key_lifecycle_http_client argument, then pass a non-None client through
at least one build_platform_app test invocation and assert the captured value is
the identical object provided. Preserve the existing test setup while ensuring
both fakes record the lifecycle client.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9239cf6f-de13-4a8e-849a-4416c4408ab3

📥 Commits

Reviewing files that changed from the base of the PR and between 60f5c8f and 9c3ecd7.

⛔ Files ignored due to path filters (17)
  • sdk/python/nemo-platform/.nmpcontext/openapi.yaml is excluded by !sdk/**
  • sdk/python/nemo-platform/.nmpcontext/stainless.yaml is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/cli/core/errors.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/access_keys/access_keys.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/access_keys/api.md is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/__init__.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/access_key_create_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/access_key_create_response.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/access_key_list_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/access_key_list_response.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/access_key_metadata_response.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/access_keys/access_key_revoke_response.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/test_access_keys.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/core/test_errors.py is excluded by !sdk/**
  • sdk/stainless.yaml is excluded by !sdk/**
📒 Files selected for processing (45)
  • docs/auth/authentication/using-authentication.mdx
  • docs/auth/deployment/configuration.mdx
  • docs/cli/reference.mdx
  • openapi/ga/individual/platform.openapi.yaml
  • openapi/ga/openapi.yaml
  • openapi/openapi.yaml
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/core/errors.py
  • packages/nemo_platform_ext/tests/cli/commands/test_auth.py
  • packages/nemo_platform_ext/tests/cli/core/test_errors.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/access_keys/client.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/access_keys/endpoints.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/access_keys/issuer.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/access_keys/types.py
  • packages/nemo_platform_plugin/tests/auth/access_keys/test_client.py
  • packages/nemo_platform_plugin/tests/auth/access_keys/test_endpoints.py
  • packages/nmp_common/src/nmp/common/auth/access_key_lifecycle.py
  • packages/nmp_common/src/nmp/common/auth/access_keys.py
  • packages/nmp_common/src/nmp/common/auth/jwt.py
  • packages/nmp_common/src/nmp/common/auth/middleware.py
  • packages/nmp_common/src/nmp/common/auth/token_resolver.py
  • packages/nmp_common/src/nmp/common/config/base.py
  • packages/nmp_common/src/nmp/common/entities/client.py
  • packages/nmp_common/tests/auth/test_access_key_lifecycle.py
  • packages/nmp_common/tests/auth/test_access_keys.py
  • packages/nmp_common/tests/auth/test_jwt.py
  • packages/nmp_common/tests/auth/test_middleware.py
  • packages/nmp_common/tests/entities/test_client.py
  • packages/nmp_platform_runner/src/nmp/platform_runner/server.py
  • packages/nmp_platform_runner/tests/test_server.py
  • packages/nmp_testing/src/nmp/testing/client.py
  • services/core/auth/src/nmp/core/auth/api/v2/access_keys/endpoints.py
  • services/core/auth/src/nmp/core/auth/api/v2/access_keys/schemas.py
  • services/core/auth/src/nmp/core/auth/api/v2/authenticate.py
  • services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py
  • services/core/auth/src/nmp/core/auth/app/access_keys.py
  • services/core/auth/src/nmp/core/auth/entities/__init__.py
  • services/core/auth/src/nmp/core/auth/entities/entities.py
  • services/core/auth/tests/integration/test_scoped_access_keys.py
  • services/core/auth/tests/test_access_key_registry.py
  • services/core/auth/tests/test_access_keys.py
  • services/core/auth/tests/test_authenticate.py
  • services/core/auth/tests/test_embedded_pdp_stress.py
  • services/core/auth/tests/test_workload_token_exchange.py
  • services/core/inference-gateway/tests/integration/test_mock_provider_mode.py
🚧 Files skipped from review as they are similar to previous changes (30)
  • services/core/auth/src/nmp/core/auth/entities/init.py
  • packages/nmp_common/src/nmp/common/config/base.py
  • services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py
  • services/core/auth/tests/test_workload_token_exchange.py
  • services/core/inference-gateway/tests/integration/test_mock_provider_mode.py
  • services/core/auth/tests/integration/test_scoped_access_keys.py
  • docs/cli/reference.mdx
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/access_keys/endpoints.py
  • docs/auth/authentication/using-authentication.mdx
  • packages/nmp_common/tests/auth/test_jwt.py
  • docs/auth/deployment/configuration.mdx
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/access_keys/issuer.py
  • packages/nemo_platform_plugin/tests/auth/access_keys/test_endpoints.py
  • packages/nmp_common/tests/entities/test_client.py
  • packages/nmp_common/src/nmp/common/entities/client.py
  • packages/nemo_platform_plugin/tests/auth/access_keys/test_client.py
  • packages/nemo_platform_ext/tests/cli/commands/test_auth.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/access_keys/client.py
  • packages/nmp_common/src/nmp/common/auth/jwt.py
  • services/core/auth/src/nmp/core/auth/api/v2/access_keys/schemas.py
  • services/core/auth/src/nmp/core/auth/api/v2/access_keys/endpoints.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/access_keys/types.py
  • services/core/auth/src/nmp/core/auth/app/access_keys.py
  • services/core/auth/src/nmp/core/auth/api/v2/authenticate.py
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/core/errors.py
  • packages/nmp_common/src/nmp/common/auth/middleware.py
  • openapi/openapi.yaml
  • openapi/ga/openapi.yaml
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
  • openapi/ga/individual/platform.openapi.yaml

@anastasia-nesterenko
anastasia-nesterenko force-pushed the anesterenko/aircore-983-implement-access-keys-revoke branch from 9c3ecd7 to ebdbf49 Compare August 11, 2026 18:24

@ironcommit ironcommit left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@anastasia-nesterenko
anastasia-nesterenko added this pull request to the merge queue Aug 11, 2026
@anastasia-nesterenko
anastasia-nesterenko removed this pull request from the merge queue due to a manual request Aug 11, 2026
@anastasia-nesterenko
anastasia-nesterenko force-pushed the anesterenko/aircore-983-implement-access-keys-revoke branch 7 times, most recently from eed507a to 63248e0 Compare August 13, 2026 02:19
Signed-off-by: anastasia-nesterenko <anesterenko@nvidia.com>
Signed-off-by: anastasia-nesterenko <anesterenko@nvidia.com>
Signed-off-by: anastasia-nesterenko <anesterenko@nvidia.com>
Signed-off-by: anastasia-nesterenko <anesterenko@nvidia.com>
Signed-off-by: anastasia-nesterenko <anesterenko@nvidia.com>
Signed-off-by: anastasia-nesterenko <anesterenko@nvidia.com>
Signed-off-by: anastasia-nesterenko <anesterenko@nvidia.com>
Signed-off-by: anastasia-nesterenko <anesterenko@nvidia.com>
@anastasia-nesterenko
anastasia-nesterenko force-pushed the anesterenko/aircore-983-implement-access-keys-revoke branch from 63248e0 to 02e326b Compare August 13, 2026 05:02
@anastasia-nesterenko
anastasia-nesterenko added this pull request to the merge queue Aug 13, 2026
Merged via the queue into main with commit 58edff5 Aug 13, 2026
57 checks passed
@anastasia-nesterenko
anastasia-nesterenko deleted the anesterenko/aircore-983-implement-access-keys-revoke branch August 13, 2026 05:46
anastasia-nesterenko added a commit that referenced this pull request Aug 13, 2026
Local and CI reproduction attempts show the worker-crash on this test is not
fixed by resetting the wasmtime policy singleton on shutdown (still crashes
with no traceback after that change), so the xdist_group mitigation and the
coverage added alongside it are reverted pending further root-causing.

Signed-off-by: anastasia-nesterenko <anesterenko@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants