packages/nvidia_nat_mcp/src/nat/plugins/mcp/auth/__init__.py (1)

1-15: LGTM on licensing; consider a minimal package docstring.

File has the correct SPDX/Apache-2.0 header. Optional: add a one‑line module docstring to describe the auth plugin package.

+# Package initializer for MCP auth plugins.
+"""MCP authentication plugin package."""

src/nat/authentication/credential_validator/__init__.py (1)

1-15: Add a brief module docstring (public module under src/).

Guidelines require Google‑style docstrings for public modules in src/. Add a concise summary.

+# Credential validator package.
+"""Credential validation primitives (e.g., bearer/JWT/introspection)."""

docs/source/workflows/mcp/mcp-server.md (1)

22-22: Use the official product name on first mention.

Replace the first occurrence with “NVIDIA NeMo Agent toolkit” per docs guidelines; use “NeMo Agent toolkit” thereafter.

-This guide will cover how to use NeMo Agent toolkit as an MCP Server to publish tools using MCP. For more information on how to use NeMo Agent toolkit as an MCP Host with one or more MCP Clients, refer to the [MCP Client](./mcp-client.md) documentation.
+This guide covers using the NVIDIA NeMo Agent toolkit as an MCP Server to publish tools using MCP. For more information on using the NeMo Agent toolkit as an MCP Host with one or more MCP Clients, refer to the [MCP Client](./mcp-client.md) documentation.

examples/MCP/simple_calculator_mcp/deploy_example_as/Dockerfile (1)

16-48: Harden the Dockerfile: avoid upgrade, run as non‑root.

• Drop apt-get upgrade -y for reproducibility.
• Add a non‑root user.
• Set noninteractive APT to avoid tzdata prompts.

-   RUN apt-get update && apt-get upgrade -y \
+   ARG DEBIAN_FRONTEND=noninteractive
+   RUN apt-get update \
     && apt-get install -y --no-install-recommends \
          python3 python3-pip git ca-certificates \
     && rm -rf /var/lib/apt/lists/*
@@
-   EXPOSE 9000
+   EXPOSE 9000
+
+   # ---- Run as non-root ----
+   RUN useradd -m -U -s /bin/bash appuser
+   USER appuser

packages/nvidia_nat_mcp/src/nat/plugins/mcp/client_impl.py (2)

61-85: Shorten error messages to satisfy Ruff TRY003 (no long exception messages).

Ruff flags these long ValueError messages. Keep messages concise or introduce a typed exception. Below shortens messages to pass TRY003.

-                raise ValueError("url should not be set when using stdio transport")
+                raise ValueError("url not allowed for stdio")
-                raise ValueError("command is required when using stdio transport")
+                raise ValueError("command required for stdio")
-                raise ValueError("Authentication is not supported for stdio transport")
+                raise ValueError("auth_provider not supported for stdio")
-                raise ValueError("command, args, and env should not be set when using sse transport")
+                raise ValueError("command/args/env not allowed for sse")
-                raise ValueError("url is required when using sse transport")
+                raise ValueError("url required for sse")
-                raise ValueError("command, args, and env should not be set when using streamable-http transport")
+                raise ValueError("command/args/env not allowed for streamable-http")
-                raise ValueError("url is required when using streamable-http transport")
+                raise ValueError("url required for streamable-http")

---

132-140: Log exceptions per guidelines; avoid returning raw error text.

Catching and returning str(e) hides stack traces and leaks messages into model outputs. Log with logger.exception() and either re‑raise or return a generic message.

-        except Exception as e:
-            return str(e)
+        except Exception:
+            logger.exception("MCP tool call failed")
+            # Prefer propagating to framework error handling:
+            raise

If callers depend on a string return, confirm and we can instead return a generic message while logging the stack trace.

docs/source/workflows/mcp/mcp-auth.md (1)

31-37: Explicitly mark the stubbed verifier as test‑only.

Add a clear warning that the stub verifier “always succeeds” is for local testing only and MUST NOT be used in production.

-This starts a protected MCP server on port 9901. This MCP server has a stub token verifier that will always return success without AS introspection.
+This starts a protected MCP server on port 9901.
+WARNING: The token verifier is a stub that always succeeds without AS introspection. For local testing only; DO NOT use in production.

docs/source/workflows/mcp/index.md (1)

20-21: Use the required first mention: “NVIDIA NeMo Agent toolkit”.

Per docs naming rule, the first mention on a page must be “NVIDIA NeMo Agent toolkit”.

Apply:

-NeMo Agent toolkit [Model Context Protocol (MCP)](https://modelcontextprotocol.io/) integration includes:
+NVIDIA NeMo Agent toolkit [Model Context Protocol (MCP)](https://modelcontextprotocol.io/) integration includes:

packages/nvidia_nat_mcp/src/nat/plugins/mcp/utils.py (1)

45-46: Tighten type hints for helper.

Be explicit about the return tuple to help pyright.

-    def _generate_field(field_name: str, field_properties: dict[str, Any]) -> tuple:
+    def _generate_field(field_name: str, field_properties: dict[str, Any]) -> tuple[Any, Field]:

examples/MCP/simple_calculator_mcp/configs/config-mcp-auth-static.yml (1)

33-46: Production safety note: use HTTPS and PKCE in real deployments.

The example is fine for local demos, but please add a note/warning about enabling HTTPS and use_pkce: true outside localhost.

Add a comment block in this file or README reminding users to switch to TLS endpoints and PKCE.

packages/nvidia_nat_mcp/src/nat/plugins/mcp/auth/register.py (1)

23-25: Fix Ruff ARG001 and add return annotation.
builder is unused; also add an explicit return type per guidelines.

-from nat.builder.builder import Builder
+from typing import AsyncIterator
+from nat.builder.builder import Builder
@@
-@register_auth_provider(config_type=MCPOAuth2ProviderConfig)
-async def mcp_oauth2_provider(authentication_provider: MCPOAuth2ProviderConfig, builder: Builder):
+@register_auth_provider(config_type=MCPOAuth2ProviderConfig)
+async def mcp_oauth2_provider(authentication_provider: MCPOAuth2ProviderConfig, _builder: Builder) -> AsyncIterator[MCPOAuth2Provider]:
     """Register MCP OAuth2 authentication provider with NAT system."""
     yield MCPOAuth2Provider(authentication_provider)

examples/MCP/simple_calculator_mcp/configs/config-mcp-server-auth.yml (1)

37-46: Examples should default to nat_test_llm for reproducibility.
Guidelines recommend stubbing LLMs in YAML. Keep real backends in a separate config if needed.

-llms:
-  nim_llm:
-    _type: nim
-    model_name: meta/llama-3.1-70b-instruct
-    temperature: 0.0
-    max_tokens: 1024
-  openai_llm:
-    _type: openai
-    model_name: gpt-3.5-turbo
-    max_tokens: 2000
+llms:
+  test_llm:
+    _type: nat_test_llm
+    response_seq:
+      - "42"
+    delay_ms: 0

If you prefer to keep real LLMs here, consider adding a parallel config-mcp-server-auth-test.yml for CI.

docs/source/workflows/mcp/mcp-client.md (2)

18-25: Use official naming on first mention.
First occurrence should be “NVIDIA NeMo Agent toolkit”; subsequent uses may say “NeMo Agent toolkit.”

-# NeMo Agent Toolkit as an MCP Client
+# NVIDIA NeMo Agent toolkit as an MCP Client
@@
-You can use NeMo Agent toolkit as an MCP Host with one or more MCP Clients serving tools from remote MCP servers.
+You can use the NVIDIA NeMo Agent toolkit as an MCP Host with one or more MCP Clients serving tools from remote MCP servers.
@@
-This guide will cover how to use NeMo Agent toolkit as an MCP Host with one or more MCP Clients. For more information on how to use NeMo Agent toolkit as an MCP Server, please refer to the [MCP Server](./mcp-server.md) documentation.
+This guide will cover how to use the NeMo Agent toolkit as an MCP Host with one or more MCP Clients. For information on using it as an MCP Server, see [MCP Server](./mcp-server.md).

---

78-83: Fix grammar and Markdown list style.
“it’s” → “its”; use dashes and proper indentation (markdownlint MD004/MD007).

-Once configured, a Pydantic input schema will be generated based on the input schema provided by the MCP server. This input schema is included with the configured function and is accessible by any agent or function calling the configured `mcp_tool_wrapper` function. The `mcp_tool_wrapper` function can accept the following type of arguments as long as they satisfy the input schema:
- * a validated instance of it's input schema
- * a string that represents a valid JSON
- * A python dictionary
- * Keyword arguments
+Once configured, a Pydantic input schema is generated from the MCP server’s input schema and attached to the function. Calls to `mcp_tool_wrapper` may pass:
+- a validated instance of its input schema
+- a string containing valid JSON
+- a Python dictionary
+- keyword arguments

src/nat/front_ends/mcp/mcp_front_end_plugin.py (1)

59-67: Make token-verifier factory synchronous and avoid redundant check.
No awaits inside; also don’t re-read self.front_end_config if the config is already passed in.

-    async def _create_token_verifier(self, token_verifier_config: OAuth2ResourceServerConfig):
-        """Create a token verifier based on configuration."""
-        from nat.front_ends.mcp.introspection_token_verifier import IntrospectionTokenVerifier
-
-        if not self.front_end_config.server_auth:
-            return None
-
-        return IntrospectionTokenVerifier(token_verifier_config)
+    def _create_token_verifier(self, token_verifier_config: OAuth2ResourceServerConfig):
+        """Create a token verifier based on configuration."""
+        from nat.front_ends.mcp.introspection_token_verifier import IntrospectionTokenVerifier
+        return IntrospectionTokenVerifier(token_verifier_config)

And update the call site accordingly (see below).

src/nat/authentication/oauth2/oauth2_resource_server_config.py (4)

16-16: Add a brief module docstring.

Public modules under src/ require a Google‑style docstring summarizing purpose and usage.

+"""OAuth 2.0 Resource Server configuration for JWT/JWKS and RFC 7662 introspection."""

---

76-88: Avoid blind except Exception in URL helper.

urlparse won’t raise; the broad catch hides bugs and triggers BLE001.

-    @staticmethod
-    def _is_https_or_localhost(url: str) -> bool:
-        try:
-            p = urlparse(url)
-            if not p.scheme or not p.netloc:
-                return False
-            if p.scheme == "https":
-                return True
-            # Allow http only for localhost-style hosts during local dev
-            return p.scheme == "http" and (p.hostname in {"localhost", "127.0.0.1", "::1"})
-        except Exception:
-            return False
+    @staticmethod
+    def _is_https_or_localhost(url: str) -> bool:
+        p = urlparse(url)
+        if not p.scheme or not p.netloc:
+            return False
+        if p.scheme == "https":
+            return True
+        # Allow http only for localhost-style hosts during local dev
+        return p.scheme == "http" and (p.hostname in {"localhost", "127.0.0.1", "::1"})

---

89-97: Annotate validator signature and return type.

Improves readability and satisfies strict type‑checking.

-from pydantic import field_validator
+from pydantic import field_validator
+from pydantic_core.core_schema import FieldValidationInfo
@@
-    def _require_valid_url(cls, v: str | None, info):
+    def _require_valid_url(cls, v: str | None, info: FieldValidationInfo) -> str | None:

---

100-129: Return Self from model validator.

Pydantic v2 “after” validators should return self; annotate it. Also import Self.

+from typing import Self
@@
-    def _ensure_verification_path(self):
+    def _ensure_verification_path(self) -> Self:
@@
-        return self
+        return self

tests/nat/authentication/test_oauth_resource_server_config.py (5)

8-24: Conform pytest fixture naming policy.

Per repo guidelines, use fixture_ prefix and name=... to expose the original fixture name.

-@pytest.fixture
-def base_config() -> OAuth2ResourceServerConfig:
+@pytest.fixture(name="base_config")
+def fixture_base_config() -> OAuth2ResourceServerConfig:

---

199-207: Silence “hardcoded secret” lint in tests.

Mark test credentials with noqa to keep ruff clean.

-        client_id="client-abc",
-        client_secret="secret-xyz",
+        client_id="client-abc",
+        client_secret="secret-xyz",  # noqa: S105

---

223-225: Silence “hardcoded secret” lint in tests (duplicate spot).

-            client_secret="secret-xyz",
+            client_secret="secret-xyz",  # noqa: S105

---

267-269: Replace curly apostrophe to satisfy RUF003.

-        "client-abc",  # we’ll set endpoint but **omit** secret to trigger invalid (missing secret)
+        "client-abc",  # we'll set endpoint but **omit** secret to trigger invalid (missing secret)

---

299-301: Replace curly apostrophe to satisfy RUF003 (second spot).

-        "secret-xyz",  # we’ll set endpoint but **omit** id to trigger invalid (missing id)
+        "secret-xyz",  # we'll set endpoint but **omit** id to trigger invalid (missing id)

tests/nat/mcp/test_mcp_auth_provider.py (4)

56-59: Silence test‑only secrets to satisfy ruff S105.

-        client_secret="test_client_secret",
+        client_secret="test_client_secret",  # noqa: S105

---

66-69: Silence S106 false‑positive for token URL literal.

-        token_url="https://auth.example.com/token",
+        token_url="https://auth.example.com/token",  # noqa: S106

---

331-334: Silence S105 on test secret (duplicate spot).

-                mock_register.return_value = OAuth2Credentials(client_id="test_client_id",
-                                                               client_secret="test_client_secret")
+                mock_register.return_value = OAuth2Credentials(client_id="test_client_id",
+                                                               client_secret="test_client_secret")  # noqa: S105

---

407-410: Silence S105 on test secret (another occurrence).

-            client_secret="test_client_secret",
+            client_secret="test_client_secret",  # noqa: S105

src/nat/front_ends/mcp/introspection_token_verifier.py (1)

34-68: Minor: tighten local types and defaults.

scopes is always a list; pass None to validator only if you want “no scope policy”. Optional.

-        scopes = config.scopes or []
+        scopes = config.scopes if config.scopes else None

tests/nat/authentication/test_bearer_token_validator.py (7)

104-121: Silence unused args warnings in fakes.

Keeps ruff clean without changing behavior.

-    def __init__(self, *args, **kwargs):
+    def __init__(self, *_: Any, **__: Any):
         self._closed = False
@@
-    async def get(self, url: str, *args, **kwargs):
+    async def get(self, url: str, *_: Any, **kwargs: Any):

---

127-144: Annotate mutable class attributes as ClassVar.

Fixes RUF012.

-from typing import Any
+from typing import Any, ClassVar
@@
-class _FakeAsyncOAuth2Client:
+class _FakeAsyncOAuth2Client:
     """Fake AsyncOAuth2Client that returns a configurable introspection response."""
-    call_count = 0
-    response: dict[str, Any] = {}
+    call_count: ClassVar[int] = 0
+    response: ClassVar[dict[str, Any]] = {}

---

141-144: Silence unused parameters in fake method.

-    async def introspect_token(self, endpoint: str, token: str, token_type_hint: str = "access_token"):
+    async def introspect_token(self, _endpoint: str, _token: str, token_type_hint: str = "access_token"):
         _FakeAsyncOAuth2Client.call_count += 1
         return _FakeAsyncOAuth2Client.response

---

210-213: Silence S105 on test client_secret.

-        client_secret="secret-xyz",
+        client_secret="secret-xyz",  # noqa: S105

---

225-228: Silence S105 on test client_secret (dup).

-        client_secret="secret-xyz",
+        client_secret="secret-xyz",  # noqa: S105

---

276-277: Silence S105 on fake opaque token string.

-    token = "opaque-secret-token-1234567890"
+    token = "opaque-secret-token-1234567890"  # noqa: S105

---

296-297: Silence S105 on fake token (dup).

-    token = "opaque-missing-scope"
+    token = "opaque-missing-scope"  # noqa: S105

examples/MCP/simple_calculator_mcp/configs/config-mcp-auth-dynamic.yml (2)

38-44: Consider making the NIM LLM model configurable.

The hardcoded model name meta/llama-3.1-70b-instruct on line 41 might become outdated.

Consider using environment variables or config templates to make the model name configurable:

 llms:
   nim_llm:
     _type: nim
-    model_name: meta/llama-3.1-70b-instruct
+    model_name: ${NIM_MODEL_NAME:-meta/llama-3.1-70b-instruct}
     temperature: 0.0
     max_tokens: 1024

---

16-31: Clarify/standardize tool_filter usage across MCP configs

Auth provider (mcp_oauth2_dynamic) looks correct.

• Observation: tool_filter is used as an exclusion list (array) in:
  • examples/MCP/simple_calculator_mcp/configs/config-mcp-auth-static.yml
  • examples/MCP/simple_calculator_mcp/configs/config-mcp-auth-dynamic.yml
    and as a mapping/override (inclusion + attribute overrides) in:
  • examples/MCP/simple_calculator_mcp/configs/config-mcp-date-stdio.yml
• Action: standardize the schema or split into explicit keys (e.g., tool_exclude vs tool_overrides) and update docs/configs accordingly.

src/nat/authentication/credential_validator/bearer_token_validator.py (3)

118-119: Strengthen JWT token detection.

Using token.count(".") == 2 for JWT detection is fragile. JWTs can have additional dots in the payload.

Apply this more robust JWT detection:

-            if token.count(".") == 2:
-                return await self._verify_jwt_token(token)
+            # JWTs have 3 parts separated by dots: header.payload.signature
+            parts = token.split(".")
+            if len(parts) == 3 and all(parts):
+                return await self._verify_jwt_token(token)

---

123-124: Avoid catching blind exceptions.

Line 123 catches all exceptions blindly, which could hide critical issues.

Be more specific about expected exceptions:

-        except Exception:
+        except (ValueError, TypeError, KeyError, httpx.HTTPError, jwt.JWTError):
+            logger.debug("Token validation failed", exc_info=True)
             return TokenValidationResult(client_id="", token_type="bearer", active=False)

---

255-258: Consider TTL bounds for cache expiration.

The cache TTL calculation could be more defensive to prevent edge cases.

Apply bounds checking to the cache TTL:

                 # Cache positive result with TTL based on token expiration
                 if expires_at:
-                    cache_expires_at = min(expires_at, int(time.time()) + 300)  # Max 5 minutes
+                    now = int(time.time())
+                    # Ensure cache doesn't outlive token and has reasonable bounds
+                    cache_expires_at = max(now + 60, min(expires_at, now + 300))  # Min 1 min, max 5 min
                     self._introspection_cache[cache_key] = {"result": result, "cache_expires_at": cache_expires_at}

packages/nvidia_nat_mcp/src/nat/plugins/mcp/auth/auth_provider_config.py (1)

56-76: Improve validation error message clarity.

The validation error message could be clearer about what combinations are valid.

Enhance the error message for better user guidance:

         # Invalid configuration
         else:
-            raise ValueError("Must provide either: "
-                             "1) enable_dynamic_registration=True (dynamic), or "
-                             "2) client_id + client_secret (hybrid)")
+            raise ValueError(
+                "Invalid MCP OAuth2 configuration. Valid options are:\n"
+                "1) Dynamic registration: enable_dynamic_registration=True, no client_id\n"
+                "2) Manual registration: client_id + client_secret (both required)\n"
+                f"Current: enable_dynamic_registration={self.enable_dynamic_registration}, "
+                f"client_id={'set' if self.client_id else 'not set'}, "
+                f"client_secret={'set' if self.client_secret else 'not set'}"
+            )

packages/nvidia_nat_mcp/src/nat/plugins/mcp/auth/auth_provider.py (2)

122-136: Improve regex robustness for header parsing.

The regex pattern could be more robust to handle edge cases in WWW-Authenticate header parsing.

Consider using a more comprehensive regex pattern:

     def _extract_from_www_authenticate_header(self, hdr: str) -> str | None:
         """Extract the resource_metadata URL from the WWW-Authenticate header."""
         import re
 
         if not hdr:
             return None
-        # resource_metadata="url" | 'url' | url (case-insensitive; stop on space/comma/semicolon)
-        m = re.search(r'(?i)\bresource_metadata\s*=\s*(?:"([^"]+)"|\'([^\']+)\'|([^\s,;]+))', hdr)
+        # Handle potential whitespace and various quote styles more robustly
+        patterns = [
+            r'(?i)\bresource_metadata\s*=\s*"([^"]+)"',  # Double quotes
+            r"(?i)\bresource_metadata\s*=\s*'([^']+)'",  # Single quotes  
+            r'(?i)\bresource_metadata\s*=\s*([^\s,;]+)',  # Unquoted
+        ]
+        for pattern in patterns:
+            m = re.search(pattern, hdr)
+            if m and m.group(1):
+                url = m.group(1).strip()
+                logger.debug("Extracted resource_metadata URL: %s", url)
+                return url
-        if not m:
-            return None
-        url = next((g for g in m.groups() if g), None)
-        if url:
-            logger.debug("Extracted resource_metadata URL: %s", url)
-        return url
+        return None

---

355-360: Verify error condition check.

The error raised at line 359 seems contradictory to the comment at line 357.

The comment says "This helper is only for non-401 flows" but the check raises an error for RETRY_AFTER_401. This appears correct, but the comment could be clearer:

     async def _perform_oauth2_flow(self, auth_request: AuthRequest | None = None) -> AuthResult:
         """Perform the OAuth2 flow using NAT OAuth2 provider."""
-        # This helper is only for non-401 flows
+        # This helper should not be called for 401 retry flows
         if auth_request and auth_request.reason == AuthReason.RETRY_AFTER_401:
             raise RuntimeError("_perform_oauth2_flow should not be called for RETRY_AFTER_401")

src/nat/front_ends/mcp/introspection_token_verifier.py (4)

29-36: Docstring style: wrap code entities in backticks.

Align with guidelines by surrounding identifiers with backticks.

Apply:

-class IntrospectionTokenVerifier(TokenVerifier):
-    """Token verifier that delegates token verification to BearerTokenValidator."""
+class IntrospectionTokenVerifier(TokenVerifier):
+    """Token verifier that delegates token verification to `BearerTokenValidator`."""
@@
-    def __init__(self, config: OAuth2ResourceServerConfig):
-        """Create IntrospectionTokenVerifier from OAuth2ResourceServerConfig.
+    def __init__(self, config: OAuth2ResourceServerConfig):
+        """Create `IntrospectionTokenVerifier` from `OAuth2ResourceServerConfig`.
@@
-            config: OAuth2ResourceServerConfig
+            config: `OAuth2ResourceServerConfig`.

---

51-53: Ruff TRY003: long exception message; prefer a short message or a custom exception.

Use a short message and introduce a dedicated exception to satisfy lint and keep guidance discoverable.

Apply:

-        if not jwt_possible and not introspection_possible:
-            raise ValueError("No valid token verification method configured. "
-                             "Either provide JWT verification (jwks_uri, discovery_url, or issuer for derived JWKS) "
-                             "or introspection (introspection_endpoint with client_id and client_secret)")
+        if not jwt_possible and not introspection_possible:
+            raise TokenVerifierConfigurationError(
+                "No viable token verification method configured; configure JWT or introspection."
+            )

And add this helper (outside the range above):

+class TokenVerifierConfigurationError(ValueError):
+    """Raised when no viable token verification method is configured."""

---

67-84: Use the logger or remove it.

Since logger is defined, add a safe debug log on success (no tokens/secrets), or drop the logger import/variable.

Apply:

         if validation_result.active:
-            return AccessToken(token=token,
+            logger.debug(
+                "MCP OAuth2 token validated (active). expires_at=%s, scopes=%d",
+                validation_result.expires_at,
+                len(validation_result.scopes or []),
+            )
+            return AccessToken(token=token,
                                expires_at=validation_result.expires_at,
                                scopes=validation_result.scopes or [],
                                client_id=validation_result.client_id or "")
         return None

---

25-25: Optional: define explicit module exports.

Expose the public API explicitly.

Apply:

 logger = logging.getLogger(__name__)
 
+__all__ = ["IntrospectionTokenVerifier"]

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

src/nat/authentication/oauth2/oauth2_resource_server_config.py (4)

74-85: Narrow exception and simplify control flow in URL helper.

Avoid blind except Exception (ruff BLE001) and make branches explicit (TRY300).

 @staticmethod
 def _is_https_or_localhost(url: str) -> bool:
-    try:
-        value = urlparse(url)
-        if not value.scheme or not value.netloc:
-            return False
-        if value.scheme == "https":
-            return True
-        return value.scheme == "http" and (value.hostname in {"localhost", "127.0.0.1", "::1"})
-    except Exception:
-        return False
+    try:
+        value = urlparse(url)
+    except ValueError:
+        return False
+    if not value.scheme or not value.netloc:
+        return False
+    if value.scheme == "https":
+        return True
+    if value.scheme == "http" and (value.hostname in {"localhost", "127.0.0.1", "::1"}):
+        return True
+    return False

---

18-21: Annotate validator info parameter for type safety.

Helps pyright and readability.

-from pydantic import Field
-from pydantic import field_validator
-from pydantic import model_validator
+from pydantic import Field, ValidationInfo
+from pydantic import field_validator
+from pydantic import model_validator
@@
-    def _require_valid_url(cls, value: str | None, info):
+    def _require_valid_url(cls, value: str | None, info: ValidationInfo):
         if value is None:
             return value
         if not cls._is_https_or_localhost(value):
             raise ValueError(f"{info.field_name} must be HTTPS (http allowed only for localhost). Got: {value}")
         return value

Also applies to: 86-94

---

92-92: Shorten raised message to satisfy ruff TRY003.

Current message is long; keep concise and actionable.

-            raise ValueError(f"{info.field_name} must be HTTPS (http allowed only for localhost). Got: {value}")
+            raise ValueError(f"{info.field_name} must be HTTPS (http allowed only for localhost)")

---

114-122: Condense multi-line ValueErrors (ruff TRY003).

Make errors concise while preserving clarity.

-                raise ValueError(
-                    f"introspection_endpoint configured but missing required credentials: {', '.join(missing)}")
+                raise ValueError("introspection_endpoint requires client_id and client_secret")
@@
-            raise ValueError("Invalid configuration: no verification method available. "
-                             "Configure one of the following:\n"
-                             "  • JWT path: set jwks_uri OR discovery_url OR issuer_url (for JWKS fallback)\n"
-                             "  • Opaque path: set introspection_endpoint + client_id + client_secret")
+            raise ValueError("No verification method: configure JWT (jwks_uri/discovery_url/issuer_url) "
+                             "or opaque (introspection_endpoint+client_id+client_secret)")

tests/nat/authentication/test_oauth_resource_server_config.py (3)

23-25: Align fixture naming with repo guidelines (name=..., function prefixed with fixture_).

Prevents CI/style failures.

-@pytest.fixture
-def base_config() -> OAuth2ResourceServerConfig:
+@pytest.fixture(name="base_config")
+def fixture_base_config() -> OAuth2ResourceServerConfig:

---

216-218: Silence test-only secret detectors (ruff S105/S106).

These are synthetic credentials in tests.

-        client_id="client-abc",
-        client_secret="secret-xyz",
+        client_id="client-abc",
+        client_secret="secret-xyz",  # noqa: S105,S106
@@
-            client_id="client-abc",
-            client_secret="secret-xyz",
+            client_id="client-abc",
+            client_secret="secret-xyz",  # noqa: S105,S106
@@
-            client_id=client_id,
-            client_secret=client_secret,
+            client_id=client_id,
+            client_secret=client_secret,  # noqa: S105,S106

Also applies to: 237-239, 257-259

---

281-282: Replace curly apostrophes in comments (ruff RUF003).

Use ASCII '.

-        "client-abc",  # we’ll set endpoint but **omit** secret to trigger invalid (missing secret)
+        "client-abc",  # we'll set endpoint but **omit** secret to trigger invalid (missing secret)
@@
-        "secret-xyz",  # we’ll set endpoint but **omit** id to trigger invalid (missing id)
+        "secret-xyz",  # we'll set endpoint but **omit** id to trigger invalid (missing id)

Also applies to: 314-316

tests/nat/authentication/test_bearer_token_validator.py (4)

16-21: Import ClassVar for mock class attributes.

Satisfies ruff RUF012.

-import time
-from typing import Any
+import time
+from typing import Any, ClassVar

---

143-146: Annotate mutable class attributes as ClassVar.

Prevents accidental instance mutation and silences ruff RUF012.

-class _MockAsyncOAuth2Client:
-
-    call_count = 0
-    response: dict[str, Any] = {}
+class _MockAsyncOAuth2Client:
+    call_count: ClassVar[int] = 0
+    response: ClassVar[dict[str, Any]] = {}

---

121-123: Mark unused args to appease ruff (ARG002) in test doubles.

No behavior change.

-    def __init__(self, *args, **kwargs):
+    def __init__(self, *_args, **_kwargs):
         self._closed = False
@@
-    async def get(self, url: str, *args, **kwargs):
+    async def get(self, url: str, *_args, **_kwargs):
@@
-    async def introspect_token(self, endpoint: str, token: str, token_type_hint: str = "access_token"):
+    async def introspect_token(self, _endpoint: str, _token: str, token_type_hint: str = "access_token"):
         _MockAsyncOAuth2Client.call_count += 1
         return _MockAsyncOAuth2Client.response

Also applies to: 130-139, 155-158

---

221-222: Silence test-only secrets/tokens (ruff S105/S106).

Inline-annotate assignments.

-        client_secret="secret-xyz",
+        client_secret="secret-xyz",  # noqa: S105,S106
@@
-        client_secret="secret-xyz",
+        client_secret="secret-xyz",  # noqa: S105,S106
@@
-    token = "opaque-secret-token-1234567890"
+    token = "opaque-secret-token-1234567890"  # noqa: S105
@@
-    non_jwt = "opaque-not-jwt-123456"
+    non_jwt = "opaque-not-jwt-123456"  # noqa: S105

Also applies to: 236-237, 287-288, 307-308

src/nat/authentication/credential_validator/bearer_token_validator.py (4)

123-127: Handle “Bearer” scheme case-insensitively (RFC 6750) and trim robustly.

Prevents false negatives when header casing varies.

-        if token.startswith("Bearer "):
-            token = token[7:]
+        if token[:7].lower() == "bearer ":
+            token = token[7:].lstrip()

---

129-137: Avoid blind except; log stack traces per guidelines and use helper _is_jwt_token.

Improves diagnosability; keeps inactive fallback.

-        try:
-            if token.count(".") == 2:
-                return await self._verify_jwt_token(token)
-            elif (self.introspection_endpoint and self.client_id and self.client_secret):
-                return await self._verify_opaque_token(token)
-            else:
-                return TokenValidationResult(client_id="", token_type="bearer", active=False)
-        except Exception:
-            return TokenValidationResult(client_id="", token_type="bearer", active=False)
+        try:
+            if self._is_jwt_token(token):
+                return await self._verify_jwt_token(token)
+            if self.introspection_endpoint and self.client_id and self.client_secret:
+                return await self._verify_opaque_token(token)
+            return TokenValidationResult(client_id="", token_type="bearer", active=False)
+        except Exception:  # noqa: BLE001
+            logger.exception("Bearer token verification failed")
+            return TokenValidationResult(client_id="", token_type="bearer", active=False)

---

107-110: Shorten long exception string (ruff TRY003).

Keep concise; same intent.

-            raise ValueError("No valid token verification method configured. "
-                             "Either provide JWT verification (jwks_uri, discovery_url, or issuer for derived JWKS) "
-                             "or introspection (introspection_endpoint with client_id and client_secret)")
+            raise ValueError("No verification configured: set jwks/discovery/issuer or introspection+client creds")

---

471-477: Condense error messages to comply with ruff TRY003.

Apply similar tightening across policy checks.

-            raise ValueError(f"JWT issuer '{issuer_claim}' does not match expected issuer '{self.issuer}'")
+            raise ValueError("JWT issuer mismatch")
@@
-                raise ValueError(f"JWT audience {audience_claim} does not contain required audience '{self.audience}'")
+                raise ValueError("JWT audience missing required value")

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-08-28T23:22:41.742Z
Learning: Applies to **/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst} : All source files must include the SPDX Apache‑2.0 header; do not bypass CI header checks

Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-08-28T23:22:41.742Z
Learning: Applies to **/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst} : Every file must start with the standard SPDX Apache-2.0 header; keep copyright years up‑to‑date

src/nat/front_ends/mcp/mcp_front_end_config.py (1)

44-45: Optional field now truly optional.

Adding default=None resolves the earlier “effectively required” issue for server_auth.

src/nat/front_ends/mcp/mcp_front_end_config.py (2)

44-45: Redact secrets from nested auth config.

server_auth can include client_secret; ensure it’s masked in logs/dumps. Prefer SecretStr in the nested model.

Apply in src/nat/authentication/oauth2/oauth2_resource_server_config.py:

-from pydantic import Field
+from pydantic import Field, SecretStr
@@
-    client_secret: str | None = Field(
+    client_secret: SecretStr | None = Field(
         default=None,
         description="OAuth2 client secret for authenticating to the introspection endpoint (opaque token validation).",
     )

If any code reads the secret, update it to call client_secret.get_secret_value().

---

16-16: Add a concise module docstring (guideline: Google‑style docstrings for public modules).

Place after the SPDX header.

Apply:

@@
-from typing import Literal
+"""MCP front end configuration and authentication wiring."""
+from typing import Literal

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: AnuradhaKaruppiah
PR: NVIDIA/NeMo-Agent-Toolkit#752
File: src/nat/front_ends/mcp/mcp_front_end_config.py:45-48
Timestamp: 2025-09-17T05:19:15.641Z
Learning: In MCPFrontEndConfig, the auth_server_url field validation is handled by a model_validator that ensures auth_server_url is provided when require_auth=True, using direct ValidationError raising.