Skip to content

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

Open
kfelternv wants to merge 3 commits into
NVIDIA:release/v2.0from
kfelternv:codex/nico-mcp-security-v2.0
Open

fix: Bind inherited MCP credentials to configured REST origin#3501
kfelternv wants to merge 3 commits into
NVIDIA:release/v2.0from
kfelternv:codex/nico-mcp-security-v2.0

Conversation

@kfelternv

@kfelternv kfelternv commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Ports the MCP credential-origin binding fix to release/v2.0.

  • Inherited credentials are forwarded only to the configured NICo REST origin.
  • An unconfigured per-call destination may proceed anonymously or with an explicit per-call token; inbound and default credentials are rejected.
  • Cross-origin redirects are rejected before credentials reach the redirect destination.
  • Equivalent configured and per-call URLs are accepted when only scheme/host casing or an explicit default HTTP(S) port differs.
  • Path, query, fragment, and user-info values remain exact.

How we verified it

Revision: 43983c5a22dd681f467d6368475b6cb898a5eca3

Environment: macOS 26.5.1 on arm64 with Go 1.26.4. The actual nico-mcp binary was built from this release PR and exercised over real localhost HTTP connections.

Before the two follow-up commits, revision aa3832b227435fc4f9b7464248ca92b94403a0cb rejected both of these equivalent pairs at the configuration gate:

  • http://LOCALHOST:19082 versus http://localhost:19082
  • http://localhost versus http://localhost:80

After the fix:

  • The case-variant call completed successfully and returned [].
  • The receiving HTTP service observed /v2/org/test-org/nico/site?pageNumber=1 and Bearer release-case-sentinel.
  • The explicit-default-port call crossed the comparison gate and attempted the request on port 80 rather than returning the base-URL mismatch error.
  • The different-path-case regression remains rejected.

Supporting checks passed:

go test ./mcp/internal/server -run TestFromCallConfig_PrecedenceChain -count=1
go test ./mcp/... -count=1
go vet ./mcp/...
gofmt -l mcp/internal/server/config.go mcp/internal/server/config_test.go mcp/internal/server/util.go
git diff --check aa3832b227435fc4f9b7464248ca92b94403a0cb..43983c5a22dd681f467d6368475b6cb898a5eca3

The gofmt -l command produced no output.

How to reproduce the verification

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

  1. Start from a clean checkout of the exact PR revision and build the binary.
git clone https://github.com/NVIDIA/infra-controller.git
cd infra-controller
git fetch origin pull/3501/head:pr-3501
git checkout pr-3501
test "$(git rev-parse HEAD)" = 43983c5a22dd681f467d6368475b6cb898a5eca3
cd rest-api
mkdir -p build
go build -o build/nico-mcp ./mcp/cmd/nico-mcp
  1. In terminal A, start a local REST receiver.
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",19082),H).serve_forever()'
  1. In terminal B, start the MCP server with the configured lowercase origin.
build/nico-mcp --listen 127.0.0.1:18082 \
  --base-url http://localhost:19082 --org test-org
  1. In terminal C, call the same origin using different hostname casing.
curl --silent --show-error \
  -H 'Content-Type: application/json' \
  -H 'Accept: application/json, text/event-stream' \
  -H 'Authorization: Bearer release-case-sentinel' \
  --data '{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"nico_get_all_site","arguments":{"base_url":"http://LOCALHOST:19082"}}}' \
  http://127.0.0.1:18082/mcp

Expected result: the MCP response contains "text":"[]", and terminal A prints:

/v2/org/test-org/nico/site?pageNumber=1 Bearer release-case-sentinel
  1. Run the focused regression table, including explicit default-port equivalence and path-case rejection.
go test ./mcp/internal/server -run TestFromCallConfig_PrecedenceChain -count=1

Expected result: the test passes.

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

copy-pr-bot Bot commented Jul 14, 2026

Copy link
Copy Markdown

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

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 880b3625-3d3b-402d-8148-6b8caa782360

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@kfelternv kfelternv requested a review from thossain-nv July 14, 2026 23:15
@kfelternv kfelternv marked this pull request as ready for review July 14, 2026 23:15
@kfelternv kfelternv requested a review from a team July 14, 2026 23:15
@kfelternv kfelternv requested a review from a team as a code owner July 14, 2026 23:15
@github-actions

Copy link
Copy Markdown

🔐 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 23:19:06 UTC | Commit: 43983c5

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
boot-artifacts-aarch64 3 0 0 3 0 0
boot-artifacts-x86_64 3 0 0 3 0 0
forge-admin-cli-x86_64 259 13 30 78 7 131
machine-validation-runner 807 40 234 288 36 209
machine_validation 807 40 234 288 36 209
machine_validation-aarch64 807 40 234 288 36 209
nvmetal-carbide 807 40 234 288 36 209
TOTAL 3493 173 966 1236 151 967

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant