fix: Bind inherited MCP credentials to configured REST origin by kfelternv · Pull Request #3499 · NVIDIA/infra-controller

kfelternv · 2026-07-14T18:29:29Z

What this PR does

Pins inherited MCP credentials to the REST origin configured when nico-mcp starts.

A caller-selected base_url must match the configured REST origin when one is set.
Without a configured REST origin, a caller-selected destination must provide its token in the same tool call; inbound and default credentials are not forwarded.
Cross-origin redirects are rejected before credentials reach the redirect destination.
Calls to the configured origin and caller-selected calls with an explicit token continue to work.

How we verified it

Revision: b45d6b3b1e3c6446a9cc8f54ba24ff01677450aa

Environment: macOS 26.5.1 on arm64 with Go 1.26.4. The standalone nico-mcp binary was built from both the PR revision and base revision 536bb264fa599d4085da8fdb66de27c953c08bd4, then exercised over real localhost HTTP connections with sentinel credentials.

Hands-on verification:

The base binary accepted a caller-selected origin and the receiving HTTP service observed Authorization: Bearer inherited-baseline-sentinel.
The PR binary rejected the same inherited-credential call with refusing to forward inherited credentials to a per-call base_url; the receiving service observed no request.
A caller-selected origin with token: explicit-head-sentinel succeeded, and the receiving service observed only that explicit token.
A configured-origin call succeeded and forwarded configured-inherited-sentinel to the matching origin.
A mismatched per-call origin was rejected with per-call base_url does not match the configured server base URL before any connection to that origin.
The base binary followed a cross-origin redirect and the redirect destination observed baseline-redirect-sentinel. The PR binary returned refusing cross-origin redirect; the redirect destination observed no request from the PR binary.

Supporting checks:

go test ./mcp/... -count=1
go vet ./mcp/...
git diff --check 536bb264fa599d4085da8fdb66de27c953c08bd4..b45d6b3b1e3c6446a9cc8f54ba24ff01677450aa

All three commands passed.

How to reproduce the verification

Prerequisites: Go 1.26.4 or newer, Python 3, and curl.

Start from a clean checkout and build the PR binary:

git clone https://github.com/NVIDIA/infra-controller.git
cd infra-controller
git fetch origin pull/3499/head:pr-3499
git checkout pr-3499
cd rest-api
mkdir -p build
go build -o build/nico-mcp ./mcp/cmd/nico-mcp

In terminal A, start a local HTTP receiver that prints the path and Authorization header:

python3 -c 'from http.server import BaseHTTPRequestHandler,HTTPServer
class H(BaseHTTPRequestHandler):
 def do_GET(self):
  print(self.path, self.headers.get("Authorization", ""), flush=True)
  body=b"[]"; self.send_response(200); self.send_header("Content-Type", "application/json"); self.send_header("Content-Length", str(len(body))); self.end_headers(); self.wfile.write(body)
 def log_message(self,*args): pass
HTTPServer(("127.0.0.1",19080),H).serve_forever()'

In terminal B, start nico-mcp without a configured REST origin:

build/nico-mcp --listen 127.0.0.1:18080

In terminal C, try to send an inherited credential to the caller-selected origin:

curl --silent --show-error \
  -H 'Content-Type: application/json' \
  -H 'Accept: application/json, text/event-stream' \
  -H 'Authorization: Bearer inherited-must-not-forward' \
  --data '{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"nico_get_all_site","arguments":{"base_url":"http://127.0.0.1:19080","org":"test-org"}}}' \
  http://127.0.0.1:18080/mcp

Expected result: the MCP response contains refusing to forward inherited credentials to a per-call base_url, and terminal A prints no new request.

Repeat with an explicit per-call token:

curl --silent --show-error \
  -H 'Content-Type: application/json' \
  -H 'Accept: application/json, text/event-stream' \
  -H 'Authorization: Bearer inherited-must-not-win' \
  --data '{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"nico_get_all_site","arguments":{"base_url":"http://127.0.0.1:19080","org":"test-org","token":"explicit-sentinel"}}}' \
  http://127.0.0.1:18080/mcp

Expected result: the MCP result contains [], and terminal A prints /v2/org/test-org/nico/site?pageNumber=1 Bearer explicit-sentinel.

Stop terminal B and restart it with the receiver as the configured origin:

build/nico-mcp --listen 127.0.0.1:18080 --base-url http://127.0.0.1:19080 --org test-org

Call it without a per-call origin:

curl --silent --show-error \
  -H 'Content-Type: application/json' \
  -H 'Accept: application/json, text/event-stream' \
  -H 'Authorization: Bearer configured-origin-sentinel' \
  --data '{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"nico_get_all_site","arguments":{}}}' \
  http://127.0.0.1:18080/mcp

Expected result: the MCP result contains [], and terminal A prints /v2/org/test-org/nico/site?pageNumber=1 Bearer configured-origin-sentinel.

Now supply a different origin:

curl --silent --show-error \
  -H 'Content-Type: application/json' \
  -H 'Accept: application/json, text/event-stream' \
  -H 'Authorization: Bearer mismatch-must-not-forward' \
  --data '{"jsonrpc":"2.0","id":4,"method":"tools/call","params":{"name":"nico_get_all_site","arguments":{"base_url":"http://127.0.0.1:19081"}}}' \
  http://127.0.0.1:18080/mcp

Expected result: the response contains per-call base_url does not match the configured server base URL; no connection to port 19081 is attempted.

Signed-off-by: Kyle Felter <kfelter@nvidia.com>

copy-pr-bot · 2026-07-14T18:29:34Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

coderabbitai · 2026-07-14T18:29:37Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 505b9474-f5cc-4f30-974b-7c58e3a1ca56

📥 Commits

Reviewing files that changed from the base of the PR and between 9ef8b43 and 4425ab0.

📒 Files selected for processing (2)

rest-api/mcp/internal/server/config_test.go
rest-api/mcp/internal/server/util.go

🚧 Files skipped from review as they are similar to previous changes (1)

rest-api/mcp/internal/server/config_test.go

Summary by CodeRabbit

Security Enhancements
- Restricted credential forwarding to trusted, configured NICo REST destinations, enforcing per-call base URL matching (with normalization).
- Rejected tool calls when destination binding doesn’t align, and prevented inherited credentials from being forwarded when unpinned.
- Blocked cross-origin redirects while allowing same-origin redirects and ensuring correct Authorization forwarding.
Documentation
- Updated Helm and CLI/MCP mode docs/help to clarify base URL binding, token precedence, and credential/config precedence.
Tests
- Added/expanded coverage for base URL matching, token forwarding, credential rejection, URL normalization, and redirect behavior.

Walkthrough

The MCP server now validates per-call destinations against a trusted configured base URL, restricts inherited credential forwarding, applies explicit token precedence, and blocks cross-origin redirects while allowing same-origin redirects. CLI, Helm, Makefile, README, and tests document and validate these rules.

Changes

MCP destination and credential policy

Layer / File(s)	Summary
Credential and destination resolution `rest-api/mcp/internal/server/config.go`, `rest-api/mcp/internal/server/config_test.go`, `rest-api/mcp/internal/server/util.go`, `rest-api/mcp/internal/server/schema.go`, `rest-api/cli/README.md`, `rest-api/cli/pkg/mcp.go`, `rest-api/mcp/cmd/nico-mcp/main.go`, `helm/rest/nico-mcp/values.yaml`, `rest-api/Makefile`	Per-call `base_url` values are normalized and matched against the configured trusted base URL. Token precedence is explicit token, inbound authorization, then configured startup credentials; inherited credentials are rejected for unpinned destinations. Documentation and CLI text reflect the updated behavior, with expanded precedence and rejection tests.
Same-origin redirect enforcement `rest-api/mcp/internal/server/server.go`, `rest-api/mcp/internal/server/server_test.go`, `rest-api/mcp/internal/server/transport_test.go`	MCP outbound clients reject cross-origin redirects and retain authorization across same-origin redirects. Origin comparison, dynamic destinations, explicit tokens, and redirect behavior are covered by tests.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MCPClient
  participant MCPToolHandler
  participant HTTPClient
  participant NICoREST
  MCPClient->>MCPToolHandler: call tool with config and credentials
  MCPToolHandler->>MCPToolHandler: resolve destination and token precedence
  MCPToolHandler->>HTTPClient: send approved REST request
  HTTPClient->>NICoREST: request trusted destination
  NICoREST-->>HTTPClient: response or redirect
  HTTPClient->>HTTPClient: allow only same-origin redirects
  HTTPClient-->>MCPToolHandler: return REST response or error
  MCPToolHandler-->>MCPClient: return tool result

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% 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
Title check	✅ Passed	The title clearly summarizes the main change: binding inherited MCP credentials to the configured REST origin.
Description check	✅ Passed	The description is directly related to the credential-origin binding changes and verification work in the PR.
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

🧪 Generate unit tests (beta)

Create PR with unit tests

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

github-actions · 2026-07-14T19:30:03Z

🔍 Container Scan Summary

Service	Total	Critical	High	Medium	Other
nico-flow	15	1	3	3	8
nico-nsm	7	0	1	5	1
nico-psm	15	1	3	3	8
nico-rest-api	17	1	4	4	8
nico-rest-cert-manager	14	1	3	3	7
nico-rest-db	15	1	3	3	8
nico-rest-site-agent	15	1	3	3	8
nico-rest-site-manager	14	1	3	3	7
nico-rest-workflow	15	1	3	3	8
TOTAL	127	8	26	30	63

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.

coderabbitai

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 `@rest-api/cli/README.md`:
- Around line 387-391: Clarify the README description of unpinned per-call
destinations: when no --base-url is configured, a per-call base_url may proceed
without credentials or with an explicit per-call token. State that only
inherited credentials—the inbound Authorization header or server default
token—are rejected rather than forwarded.

In `@rest-api/mcp/internal/server/config.go`:
- Around line 77-109: Update resolvedConfig.FromCallConfig to canonicalize the
parsed configuredBaseURL and callBaseURL components before comparing them,
including scheme and host casing while preserving path and other relevant URL
components. Use the canonical comparison for the mismatch check, while retaining
the existing normalized values for assignment and validation.

🪄 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: 5133d5a2-7f69-41fe-a173-6d226a475951

📥 Commits

Reviewing files that changed from the base of the PR and between 536bb26 and b45d6b3.

📒 Files selected for processing (11)

helm/rest/nico-mcp/values.yaml
rest-api/Makefile
rest-api/cli/README.md
rest-api/cli/pkg/mcp.go
rest-api/mcp/cmd/nico-mcp/main.go
rest-api/mcp/internal/server/config.go
rest-api/mcp/internal/server/config_test.go
rest-api/mcp/internal/server/schema.go
rest-api/mcp/internal/server/server.go
rest-api/mcp/internal/server/server_test.go
rest-api/mcp/internal/server/transport_test.go

coderabbitai

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 `@rest-api/mcp/internal/server/util.go`:
- Around line 31-45: The sameBaseURL function currently compares serialized
URLs, so equivalent default-port destinations are rejected. Update sameBaseURL
to reuse sameOrigin’s origin comparison and effectivePort handling, then
separately compare the parsed base-path components while preserving invalid-URL
rejection.

🪄 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: b8f098c1-3808-40bb-a141-284f2a031730

📥 Commits

Reviewing files that changed from the base of the PR and between b45d6b3 and 9ef8b43.

📒 Files selected for processing (4)

rest-api/cli/README.md
rest-api/mcp/internal/server/config.go
rest-api/mcp/internal/server/config_test.go
rest-api/mcp/internal/server/util.go

🚧 Files skipped from review as they are similar to previous changes (3)

rest-api/mcp/internal/server/config_test.go
rest-api/mcp/internal/server/config.go
rest-api/cli/README.md

github-actions · 2026-07-14T22:17:34Z

🔐 TruffleHog Secret Scan

✅ No secrets or credentials found!

Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉

🔗 View scan details

_{🕐 Last updated: 2026-07-14 22:17:33 UTC | Commit: 9ef8b43}

fix: Bind inherited MCP credentials to configured REST origin

b45d6b3

Signed-off-by: Kyle Felter <kfelter@nvidia.com>

kfelternv requested a review from thossain-nv July 14, 2026 19:28

kfelternv marked this pull request as ready for review July 14, 2026 19:28

kfelternv requested review from a team as code owners July 14, 2026 19:28

coderabbitai Bot reviewed Jul 14, 2026

View reviewed changes

Comment thread rest-api/cli/README.md Outdated

Comment thread rest-api/mcp/internal/server/config.go

fix: Compare configured MCP base URLs canonically

9ef8b43

coderabbitai Bot reviewed Jul 14, 2026

View reviewed changes

Comment thread rest-api/mcp/internal/server/util.go

fix: Treat explicit default MCP ports as equivalent

4425ab0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Bind inherited MCP credentials to configured REST origin#3499

fix: Bind inherited MCP credentials to configured REST origin#3499
kfelternv wants to merge 3 commits into
NVIDIA:mainfrom
kfelternv:codex/nico-mcp-security-main

kfelternv commented Jul 14, 2026 •

edited

Loading

Uh oh!

copy-pr-bot Bot commented Jul 14, 2026

Uh oh!

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Jul 14, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

kfelternv commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

How we verified it

How to reproduce the verification

Uh oh!

copy-pr-bot Bot commented Jul 14, 2026

Uh oh!

coderabbitai Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

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

Uh oh!

🔍 Container Scan Summary

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Jul 14, 2026

🔐 TruffleHog Secret Scan

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

kfelternv commented Jul 14, 2026 •

edited

Loading

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading

github-actions Bot commented Jul 14, 2026 •

edited

Loading