Skip to content

Add Cloud Governance AI Agent (MCP-based OpenSearch query interface)#997

Draft
pragya811 wants to merge 1 commit into
mainfrom
cg-mcp
Draft

Add Cloud Governance AI Agent (MCP-based OpenSearch query interface)#997
pragya811 wants to merge 1 commit into
mainfrom
cg-mcp

Conversation

@pragya811
Copy link
Copy Markdown
Member

@pragya811 pragya811 commented May 12, 2026

Type of change

Note: Fill x in []

  • bug
  • enhancement
  • documentation
  • dependencies

Summary

  • Adds a Streamlit-based AI agent that provides a natural language interface to query cloud governance data in OpenSearch
  • Uses a custom MCP (Model Context Protocol) server with high-level query tools that abstract away raw OpenSearch Query DSL
  • AI (Google Gemini) calls simple tools like search_documents(filters=[...]) and count_by_field(group_by="policy") instead of constructing complex JSON queries
  • The MCP server auto-handles .keyword field resolution, type coercion, and case-insensitive field matching
  • Includes a Dockerfile for containerized deployment and a sidebar index selector dropdown

Architecture

Streamlit UI → Gemini AI → MCP stdio → mcp_server.py subprocess → OpenSearch

Tools Available

Tool Description
list_indices List all OpenSearch indices with doc counts
get_fields Discover field names, types, and aggregatability
search_documents Filtered search with auto .keyword handling
count_by_field Group-by aggregation (terms)
aggregate Metric aggregation (sum/avg/max/min) grouped by field
date_range_search Time-range search with text/date field support
raw_search Escape hatch for raw Query DSL

Setup

cd cloud-governance-mcp
cp .env.example .env  # configure GEMINI_API_KEY, OPENSEARCH_HOSTS
python3 -m venv .venv && source .venv/bin/activate
pip install -r requirements.txt
streamlit run app.py

Test plan

- Configure .env with valid OpenSearch host and Gemini API key
- Run streamlit run app.py and verify 7 tools load
- Test: "What fields are available in this index?"
- Test: "Show me 5 sample documents"
- Test: "Count documents by [field]"
- Test aggregations with filters
- Switch index via sidebar dropdown and verify session resets

## For security reasons, all pull requests need to be approved first before running any automated CI


<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **New Features**
* Launched Cloud Governance AI Agent—an interactive application enabling natural language queries against cloud governance data with conversation history persistence and multi-index selection support.

* **Documentation**
* Added comprehensive README with quick start guide, troubleshooting steps, and security guidelines.

* **Chores**
* Added Docker containerization support, deployment script, and environment configuration templates.

<!-- review_stack_entry_start -->

[![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/redhat-performance/cloud-governance/pull/997?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 12, 2026

Important

Review skipped

Draft detected.

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fa88526b-88fb-414f-afae-d5ff958b7829

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:

  • ✅ Review completed - (🔄 Check again to review again)
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cg-mcp

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Member

@ebattat ebattat left a comment

Choose a reason for hiding this comment

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

Maybe we can limit it only to the indexes that we are using, instead of all the existing indexes. Also, it would be great if you could add a filter to the index list.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

🤖 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 `@cloud-governance-mcp/app.py`:
- Line 251: The st.error call uses an unnecessary f-string: change the call to
st.error("❌ Failed to connect to MCP server") (remove the leading f) so the
literal isn't treated as an f-string; update the string in the existing st.error
invocation where the message "❌ Failed to connect to MCP server" is used.
- Around line 336-345: The current bare "except:" around the json.loads(result)
block should be replaced with a specific exception handler for JSON parsing
(e.g., catch json.JSONDecodeError or ValueError) so non-JSON parsing failures
aren't swallowed; update the try/except that surrounds json.loads(result) to
"except json.JSONDecodeError:" (import JSONDecodeError if needed) and keep the
existing fallback behavior that truncates and calls st.text(result) when parsing
fails.
- Line 350: The function signature for run_agent_loop_gemini uses an implicit
optional type for previous_messages ("list = None"); update the parameter
annotation to explicitly show nullability by changing previous_messages to use
an explicit union with None (e.g., previous_messages: list | None = None) so the
type hint conforms to PEP 484; modify the function definition of
run_agent_loop_gemini accordingly and update any callers or imports if they rely
on a different typing style.

In `@cloud-governance-mcp/Dockerfile`:
- Line 6: The Dockerfile currently writes a Streamlit config with
showErrorDetails enabled via the printf command that contains
"[client]\nshowErrorDetails = true"; change that default to false by updating
the printf invocation (the line that writes to /root/.streamlit/config.toml) so
the file contains "showErrorDetails = false" instead of "true".
- Around line 1-15: The Dockerfile currently runs as root; create and switch to
a non-root user before CMD by adding a user/group (e.g., appuser), chowning
WORKDIR and any config files (like /root/.streamlit or move config to
/home/appuser/.streamlit) and then set USER appuser so that the final CMD
["streamlit", "run", "app.py", ...] executes unprivileged; update any file paths
or ownership for files copied (app.py, mcp_server.py, requirements) and ensure
the home/config directory exists and is owned by the new user before switching.

In `@cloud-governance-mcp/mcp_server.py`:
- Around line 619-622: The _tool_raw_search function accepts arbitrary Query
DSL; validate and sanitize query_body before calling _get_client().search by (1)
enforcing a capped size: if query_body.get("size") is missing or >1000, set it
to 1000; (2) reject or remove dangerous keys such as "script", "script_fields",
"scroll", and any top-level "stored_fields" that could cause heavy work; (3)
block requests containing "from" values greater than a safe threshold (e.g.,
10000) or convert them to use safe pagination; and (4) pass a client.search
timeout (e.g., timeout="30s" or request_timeout) to the call in _tool_raw_search
to bound execution time. Apply these checks in _tool_raw_search before calling
client.search and return a clear error message when input is rejected.

In `@cloud-governance-mcp/README.md`:
- Around line 147-149: The README currently suggests running the literal command
"cat .env | grep OPENSEARCH" which can expose secrets; replace that step with a
safer approach that only reveals variable names or redacts values—i.e., locate
lines matching OPENSEARCH but then remove or mask the value portion instead of
printing the raw .env contents (use standard text-processing tools such as grep
combined with a value-strip or redaction step, or output only the variable names
with cut), and update the README to show that safer command in place of the
existing one.

In `@cloud-governance-mcp/requirements.txt`:
- Line 3: Replace the requirements entry that currently reads "mcp[all]>=1.0.0"
with a pinned stdio-only dependency "mcp>=1.23.0,<2"; specifically update the
token mcp[all]>=1.0.0 in the requirements file to mcp>=1.23.0,<2 to remove
extras used only by non-stdio transports, tighten the minimum version to 1.23.0
for security fixes, and add the <2 upper bound for reproducible installs.

In `@cloud-governance-mcp/run_agent.sh`:
- Around line 41-43: The configuration snippet in run_agent.sh currently sets
showErrorDetails = true which exposes internal traces; change this default to
showErrorDetails = false in the [client] block (replace the true value with
false) so detailed error traces are disabled by default; ensure any comments or
downstream logic that relies on showErrorDetails are updated to expect false as
the safe default.
- Line 33: Replace the unsafe hard-kill line `kill -9 $(lsof -ti tcp:8501)` with
a graceful shutdown sequence: use `lsof -ti tcp:8501` to capture PIDs, send
SIGTERM (kill -15) to those PIDs, wait a short interval and re-check lsof; only
if any PIDs remain, escalate to SIGKILL (kill -9) for those remaining PIDs. Also
consider filtering the PIDs by expected command/user before sending signals to
avoid killing unrelated processes on port 8501.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d1b2ebb5-f92c-4ad4-b4d2-9530bc506420

📥 Commits

Reviewing files that changed from the base of the PR and between d307bce and 2f87ea3.

📒 Files selected for processing (9)
  • cloud-governance-mcp/.dockerignore
  • cloud-governance-mcp/.env.example
  • cloud-governance-mcp/.gitignore
  • cloud-governance-mcp/Dockerfile
  • cloud-governance-mcp/README.md
  • cloud-governance-mcp/app.py
  • cloud-governance-mcp/mcp_server.py
  • cloud-governance-mcp/requirements.txt
  • cloud-governance-mcp/run_agent.sh

thread.join()

if error:
st.error(f"❌ Failed to connect to MCP server")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove extraneous f prefix from string literal.

This f-string has no placeholders, making the f prefix unnecessary.

🔧 Proposed fix
-        st.error(f"❌ Failed to connect to MCP server")
+        st.error("❌ Failed to connect to MCP server")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
st.error(f"❌ Failed to connect to MCP server")
st.error("❌ Failed to connect to MCP server")
🧰 Tools
🪛 Ruff (0.15.12)

[error] 251-251: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 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 `@cloud-governance-mcp/app.py` at line 251, The st.error call uses an
unnecessary f-string: change the call to st.error("❌ Failed to connect to MCP
server") (remove the leading f) so the literal isn't treated as an f-string;
update the string in the existing st.error invocation where the message "❌
Failed to connect to MCP server" is used.

Comment on lines +336 to +345
try:
# Try to pretty-print JSON
parsed = json.loads(result)
st.code(json.dumps(parsed, indent=2), language="json")
except:
# If not JSON, show as text (truncate if too long)
if len(result) > 2000:
st.text(result[:2000] + f"\n\n... (truncated, total {len(result)} chars)")
else:
st.text(result)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Replace bare except with specific exception type.

Bare except: catches all exceptions including SystemExit and KeyboardInterrupt, which can mask critical errors. Since this is handling JSON parsing, use a specific exception.

🔧 Proposed fix
             try:
                 # Try to pretty-print JSON
                 parsed = json.loads(result)
                 st.code(json.dumps(parsed, indent=2), language="json")
-            except:
+            except (json.JSONDecodeError, TypeError):
                 # If not JSON, show as text (truncate if too long)
                 if len(result) > 2000:
                     st.text(result[:2000] + f"\n\n... (truncated, total {len(result)} chars)")
                 else:
                     st.text(result)
🧰 Tools
🪛 Ruff (0.15.12)

[error] 340-340: Do not use bare except

(E722)

🤖 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 `@cloud-governance-mcp/app.py` around lines 336 - 345, The current bare
"except:" around the json.loads(result) block should be replaced with a specific
exception handler for JSON parsing (e.g., catch json.JSONDecodeError or
ValueError) so non-JSON parsing failures aren't swallowed; update the try/except
that surrounds json.loads(result) to "except json.JSONDecodeError:" (import
JSONDecodeError if needed) and keep the existing fallback behavior that
truncates and calls st.text(result) when parsing fails.

return result


def run_agent_loop_gemini(user_message: str, tools: List[types.FunctionDeclaration], previous_messages: list = None) -> str:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use explicit | None for optional parameter type hint.

PEP 484 prohibits implicit Optional. The type hint list = None should explicitly indicate nullability.

🔧 Proposed fix
-def run_agent_loop_gemini(user_message: str, tools: List[types.FunctionDeclaration], previous_messages: list = None) -> str:
+def run_agent_loop_gemini(user_message: str, tools: List[types.FunctionDeclaration], previous_messages: list | None = None) -> str:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def run_agent_loop_gemini(user_message: str, tools: List[types.FunctionDeclaration], previous_messages: list = None) -> str:
def run_agent_loop_gemini(user_message: str, tools: List[types.FunctionDeclaration], previous_messages: list | None = None) -> str:
🧰 Tools
🪛 Ruff (0.15.12)

[warning] 350-350: PEP 484 prohibits implicit Optional

Convert to T | None

(RUF013)

🤖 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 `@cloud-governance-mcp/app.py` at line 350, The function signature for
run_agent_loop_gemini uses an implicit optional type for previous_messages
("list = None"); update the parameter annotation to explicitly show nullability
by changing previous_messages to use an explicit union with None (e.g.,
previous_messages: list | None = None) so the type hint conforms to PEP 484;
modify the function definition of run_agent_loop_gemini accordingly and update
any callers or imports if they rely on a different typing style.

Comment on lines +1 to +15
FROM python:3.11-slim

WORKDIR /app

RUN mkdir -p /root/.streamlit && \
printf '[browser]\ngatherUsageStats = false\n[client]\nshowErrorDetails = true\n' > /root/.streamlit/config.toml

COPY requirements.txt .
RUN pip install --no-cache-dir -r requirements.txt

COPY app.py mcp_server.py ./

EXPOSE 8501

CMD ["streamlit", "run", "app.py", "--server.port=8501", "--server.address=0.0.0.0", "--server.headless=true"]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Run the container as a non-root user.

The image currently runs as root; switch to an unprivileged user before CMD.

Suggested fix
 FROM python:3.11-slim
 
 WORKDIR /app
+RUN useradd -m -u 10001 appuser
 
 RUN mkdir -p /root/.streamlit && \
     printf '[browser]\ngatherUsageStats = false\n[client]\nshowErrorDetails = true\n' > /root/.streamlit/config.toml
@@
 COPY app.py mcp_server.py ./
+RUN chown -R appuser:appuser /app
+USER appuser
🧰 Tools
🪛 Trivy (0.69.3)

[error] 1-1: Image user should not be 'root'

Specify at least 1 USER command in Dockerfile with non-root user as argument

Rule: DS-0002

Learn more

(IaC/Dockerfile)


[error] 8-15: 'apt-get' missing '--no-install-recommends'

'--no-install-recommends' flag is missed: 'apt-get update && apt-get install -y wget build-essential python3-dev libldap2-dev libsasl2-dev vim && export VER=${gitleaks_version} && wget https://github.com/zricethezav/gitleaks/releases/download/v$VER/gitleaks-linux-amd64 && mv gitleaks-linux-amd64 gitleaks && chmod +x gitleaks && mv gitleaks /usr/local/bin/ && gitleaks --version'

Rule: DS-0029

Learn more

(IaC/Dockerfile)


[error] 1-1: Image user should not be 'root'

Specify at least 1 USER command in Dockerfile with non-root user as argument

Rule: DS-0002

Learn more

(IaC/Dockerfile)

🤖 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 `@cloud-governance-mcp/Dockerfile` around lines 1 - 15, The Dockerfile
currently runs as root; create and switch to a non-root user before CMD by
adding a user/group (e.g., appuser), chowning WORKDIR and any config files (like
/root/.streamlit or move config to /home/appuser/.streamlit) and then set USER
appuser so that the final CMD ["streamlit", "run", "app.py", ...] executes
unprivileged; update any file paths or ownership for files copied (app.py,
mcp_server.py, requirements) and ensure the home/config directory exists and is
owned by the new user before switching.

WORKDIR /app

RUN mkdir -p /root/.streamlit && \
printf '[browser]\ngatherUsageStats = false\n[client]\nshowErrorDetails = true\n' > /root/.streamlit/config.toml
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not enable Streamlit error details in image default config.

Set showErrorDetails = false to reduce accidental disclosure of internals in deployed environments.

🤖 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 `@cloud-governance-mcp/Dockerfile` at line 6, The Dockerfile currently writes a
Streamlit config with showErrorDetails enabled via the printf command that
contains "[client]\nshowErrorDetails = true"; change that default to false by
updating the printf invocation (the line that writes to
/root/.streamlit/config.toml) so the file contains "showErrorDetails = false"
instead of "true".

Comment on lines +619 to +622
def _tool_raw_search(index: str, query_body: dict) -> str:
client = _get_client()
response = client.search(index=index, body=query_body)
return json.dumps(response, indent=2, default=str)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoff

Consider adding safeguards to raw_search to prevent resource exhaustion.

This tool accepts arbitrary OpenSearch Query DSL without any validation. A malicious or manipulated prompt could cause the model to execute expensive queries (deep pagination with from: 100000, unbounded aggregations, or script queries) leading to cluster performance degradation. Consider:

  1. Enforcing a maximum size limit (e.g., cap at 1000)
  2. Blocking potentially expensive operations like script queries or scroll
  3. Adding a timeout to the query
🛡️ Proposed minimal safeguard
 def _tool_raw_search(index: str, query_body: dict) -> str:
     client = _get_client()
+    # Enforce size limit to prevent unbounded result fetching
+    if "size" not in query_body:
+        query_body["size"] = 100
+    elif query_body.get("size", 0) > 1000:
+        query_body["size"] = 1000
     response = client.search(index=index, body=query_body)
     return json.dumps(response, indent=2, default=str)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cloud-governance-mcp/mcp_server.py` around lines 619 - 622, The
_tool_raw_search function accepts arbitrary Query DSL; validate and sanitize
query_body before calling _get_client().search by (1) enforcing a capped size:
if query_body.get("size") is missing or >1000, set it to 1000; (2) reject or
remove dangerous keys such as "script", "script_fields", "scroll", and any
top-level "stored_fields" that could cause heavy work; (3) block requests
containing "from" values greater than a safe threshold (e.g., 10000) or convert
them to use safe pagination; and (4) pass a client.search timeout (e.g.,
timeout="30s" or request_timeout) to the call in _tool_raw_search to bound
execution time. Apply these checks in _tool_raw_search before calling
client.search and return a clear error message when input is rejected.

Comment on lines +147 to +149
# Check .env file
cat .env | grep OPENSEARCH
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid commands that print secrets from .env.

cat .env | grep OPENSEARCH may expose OPENSEARCH_PASSWORD. Use a safer command that excludes secret values.

Suggested fix
-# Check .env file
-cat .env | grep OPENSEARCH
+# Check non-secret OpenSearch settings
+grep -E '^(OPENSEARCH_HOSTS|OPENSEARCH_USERNAME)=' .env
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Check .env file
cat .env | grep OPENSEARCH
```
# Check non-secret OpenSearch settings
grep -E '^(OPENSEARCH_HOSTS|OPENSEARCH_USERNAME)=' .env
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cloud-governance-mcp/README.md` around lines 147 - 149, The README currently
suggests running the literal command "cat .env | grep OPENSEARCH" which can
expose secrets; replace that step with a safer approach that only reveals
variable names or redacts values—i.e., locate lines matching OPENSEARCH but then
remove or mask the value portion instead of printing the raw .env contents (use
standard text-processing tools such as grep combined with a value-strip or
redaction step, or output only the variable names with cut), and update the
README to show that safer command in place of the existing one.

@@ -0,0 +1,8 @@
elasticsearch>=8.10.0
google-genai>=0.3.0
mcp[all]>=1.0.0
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

For the Python MCP SDK, what is the minimum package/extras needed for stdio transport only, and which versions include fixes for GHSA-9h52-p55h-vw2f and GHSA-j975-95f5-7wqh?

💡 Result:

For the Python MCP SDK, the minimum package required is simply the mcp package itself [1][2]. No additional extras are strictly necessary for stdio transport, as the stdio transport layer is included in the core library [3][4]. Regarding the specified security vulnerabilities: 1. GHSA-9h52-p55h-vw2f (CVE-2025-66416): This vulnerability relates to a lack of DNS rebinding protection in HTTP-based MCP servers [5][6]. It is fixed in mcp version 1.23.0 and later [7][6]. Note that this specific vulnerability does not affect servers using the stdio transport [6]. 2. GHSA-j975-95f5-7wqh (CVE-2025-53365): This vulnerability involves an unhandled exception in the streamable HTTP transport that could lead to a denial of service [8][9]. It is addressed in subsequent updates to the SDK [9]. Similar to the previous issue, this vulnerability is specific to the streamable HTTP transport and does not affect stdio transport implementations [9]. To ensure your environment is secure, it is recommended to use mcp version 1.23.0 or higher [7][6].

Citations:


Change mcp[all]>=1.0.0 to mcp>=1.23.0,<2 for stdio-only usage.

For stdio transport, the base mcp package is sufficient—no extras are required. Omit [all] to eliminate unnecessary transitive dependencies and reduce attack surface. Additionally, the version constraint >=1.0.0 is too loose and lacks an upper bound; pin to version 1.23.0 or higher to capture security fixes and set an upper bound for reproducibility.

🤖 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 `@cloud-governance-mcp/requirements.txt` at line 3, Replace the requirements
entry that currently reads "mcp[all]>=1.0.0" with a pinned stdio-only dependency
"mcp>=1.23.0,<2"; specifically update the token mcp[all]>=1.0.0 in the
requirements file to mcp>=1.23.0,<2 to remove extras used only by non-stdio
transports, tighten the minimum version to 1.23.0 for security fixes, and add
the <2 upper bound for reproducible installs.


# Kill existing Streamlit processes on port 8501
echo "Stopping existing Streamlit processes..."
kill -9 $(lsof -ti tcp:8501) 2>/dev/null || true
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not hard-kill arbitrary process on port 8501.

kill -9 $(lsof -ti tcp:8501) is unsafe and can kill unrelated processes abruptly. Prefer graceful termination and only escalate if needed.

Suggested fix
- kill -9 $(lsof -ti tcp:8501) 2>/dev/null || true
+ pids="$(lsof -ti tcp:8501 || true)"
+ if [ -n "$pids" ]; then
+   kill $pids 2>/dev/null || true
+   sleep 1
+   kill -9 $pids 2>/dev/null || true
+ fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
kill -9 $(lsof -ti tcp:8501) 2>/dev/null || true
pids="$(lsof -ti tcp:8501 || true)"
if [ -n "$pids" ]; then
kill $pids 2>/dev/null || true
sleep 1
kill -9 $pids 2>/dev/null || true
fi
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 33-33: Quote this to prevent word splitting.

(SC2046)

🤖 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 `@cloud-governance-mcp/run_agent.sh` at line 33, Replace the unsafe hard-kill
line `kill -9 $(lsof -ti tcp:8501)` with a graceful shutdown sequence: use `lsof
-ti tcp:8501` to capture PIDs, send SIGTERM (kill -15) to those PIDs, wait a
short interval and re-check lsof; only if any PIDs remain, escalate to SIGKILL
(kill -9) for those remaining PIDs. Also consider filtering the PIDs by expected
command/user before sending signals to avoid killing unrelated processes on port
8501.

Comment on lines +41 to +43
[client]
showErrorDetails = true
EOF
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Disable detailed error traces by default.

showErrorDetails = true can expose internal stack traces/config in the UI. Default should be false for safer deployment posture.

🤖 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 `@cloud-governance-mcp/run_agent.sh` around lines 41 - 43, The configuration
snippet in run_agent.sh currently sets showErrorDetails = true which exposes
internal traces; change this default to showErrorDetails = false in the [client]
block (replace the true value with false) so detailed error traces are disabled
by default; ensure any comments or downstream logic that relies on
showErrorDetails are updated to expect false as the safe default.

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

Labels

enhancement New feature or request

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

2 participants