Skip to content

Staging - #2

Merged
Jeanm2005 merged 5 commits into
mainfrom
staging
Jul 23, 2026
Merged

Staging#2
Jeanm2005 merged 5 commits into
mainfrom
staging

Conversation

@Jeanm2005

@Jeanm2005 Jeanm2005 commented Jul 23, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Tests

    • Added automated validation for pushes and pull requests across supported release branches.
    • Expanded checks for proxy behavior, tool availability, schema integrity, and detection results.
    • Standardized test connection setup for more consistent execution.
  • Chores

    • Added automated linting to help maintain code quality.
    • Updated tooling requirements to support lint checks.
  • Refactor

    • Simplified proxy and test connection management without changing runtime behavior.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The pull request adds GitHub Actions CI with Ruff, three test scripts, and database assertions. It also standardizes MCP async context management in the proxy and tests, plus minor import formatting updates.

Changes

CI and async context refactor

Layer / File(s) Summary
CI workflow and lint setup
.github/workflows/ci.yml, requirements.txt, vulnerable-server/server.py
CI runs on selected branches, installs Ruff, executes the test scripts, validates database findings, and applies import ordering.
MCP context management updates
proxy/proxy.py, tests/smoke_test.py, tests/test_proxy_e2e.py, tests/test_rugpull_schema.py
Proxy and test connections use combined async context managers while preserving initialization, tool listing, proxy execution, and tool calls.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is vague and reads like a branch name rather than describing the actual code and test changes. Use a concise title that names the main change, such as adding CI workflow and test updates for the proxy.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch staging

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.

@Jeanm2005
Jeanm2005 merged commit 92bd8ca into main Jul 23, 2026
2 of 3 checks passed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
requirements.txt (1)

2-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Pin the Ruff version used by CI.

ruff>=0.6 permits future releases to change lint rules or behavior unexpectedly. Pin the tested version, or use a constraints/lock file, and update it deliberately.

🤖 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 `@requirements.txt` around lines 2 - 3, Update the Ruff dependency declaration
in requirements.txt to use a pinned, tested version instead of the open-ended
“ruff>=0.6” constraint. Keep the selected version consistent with the version
used by CI and ensure future Ruff upgrades are made deliberately.
🤖 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 @.github/workflows/ci.yml:
- Around line 9-14: Harden the test job by declaring read-only repository
permissions with contents: read and configure actions/checkout@v4 with
persist-credentials disabled. Apply these changes to the workflow-level or
job-level permissions and the checkout step, preserving the existing test steps.

---

Nitpick comments:
In `@requirements.txt`:
- Around line 2-3: Update the Ruff dependency declaration in requirements.txt to
use a pinned, tested version instead of the open-ended “ruff>=0.6” constraint.
Keep the selected version consistent with the version used by CI and ensure
future Ruff upgrades are made deliberately.
🪄 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: a2b2059f-95a0-4b95-a2c4-802f94e57b70

📥 Commits

Reviewing files that changed from the base of the PR and between 495a4ce and 3312ee2.

📒 Files selected for processing (7)
  • .github/workflows/ci.yml
  • proxy/proxy.py
  • requirements.txt
  • tests/smoke_test.py
  • tests/test_proxy_e2e.py
  • tests/test_rugpull_schema.py
  • vulnerable-server/server.py

Comment thread .github/workflows/ci.yml
Comment on lines +9 to +14
jobs:
test:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== workflow file =="
if [ -f .github/workflows/ci.yml ]; then
  cat -n .github/workflows/ci.yml
else
  echo "missing .github/workflows/ci.yml"
fi

echo
echo "== workflow files =="
fd -a '^[^.].*\.ya?ml$' .github/workflows 2>/dev/null | sed 's#^\./##' || true

echo
echo "== checkout usages in workflows =="
rg -n "actions/checkout|GITHUB_TOKEN|permissions:|persist-credentials" .github/workflows || true

echo
echo "== actionlint/config if present =="
fd -a 'actionlint|\.github' . | sed 's#^\./##' | head -100

Repository: Jeanm2005/MCP-security-proxy

Length of output: 2606


🌐 Web query:

GitHub Actions pull_request token permissions permissions contents read actions/checkout persist-credentials

💡 Result:

In GitHub Actions, the actions/checkout action allows you to check out your repository code into the runner [1][2]. The persist-credentials input controls whether the authentication token or SSH key used for checkout is saved into the local Git configuration [1][3]. Key technical details: 1. Default Behavior: The persist-credentials input defaults to true [1][4][5]. When enabled, the action configures the runner's Git environment with the credentials, which allows subsequent steps in the same job to run authenticated Git commands (such as git fetch or git push) [1][6][3]. 2. Security Implications: Because the default is true, the credentials remain available to all subsequent steps in the job until the job completes [7][6]. If your workflow does not require authenticated Git operations after the checkout step, it is a recommended security practice to explicitly set persist-credentials: false to prevent other steps (or third-party actions) from potentially accessing or misusing the token [7][8][6]. 3. Recommended Permissions: When using actions/checkout, GitHub recommends setting the GITHUB_TOKEN permissions to contents: read to ensure the action functions with the least privilege necessary [1][9][10]. If your workflow requires the ability to push commits back to the repository, you would need to set contents: write and keep persist-credentials: true [6]. To implement this security hardening, your workflow configuration should look like this [6]: jobs: job_name: permissions: contents: read steps: - uses: actions/checkout@v4 with: persist-credentials: false If you specifically need to push changes, use: jobs: job_name: permissions: contents: write steps: - uses: actions/checkout@v4 with: persist-credentials: true - run: | #... git configuration and push commands...

Citations:


🌐 Web query:

GitHub Actions GITHUB_TOKEN permissions for pull_request fork repository write permissions

💡 Result:

By default, GitHub Actions workflows triggered by pull requests from forks are granted a GITHUB_TOKEN with read-only permissions, regardless of any permissions explicitly defined in your workflow file [1][2][3]. This is a security measure designed to prevent unauthorized code in a fork from accessing or modifying your repository's secrets or contents [4]. However, there are two primary ways to manage or modify this behavior: 1. Repository Settings for Write Access: Repository administrators can enable the option to Send write tokens to workflows from pull requests in the repository's GitHub Actions settings [5][6]. When this is enabled, it allows workflows triggered by fork pull requests to receive a GITHUB_TOKEN with write permissions [5]. 2. Using the pull_request_target Trigger: You can use the pull_request_target event trigger instead of the standard pull_request trigger [3][4]. Workflows triggered by pull_request_target run in the context of the base repository and have access to secrets and a GITHUB_TOKEN with read/write permissions [4]. - Security Warning: Because pull_request_target executes using the base repository's code, you must be extremely careful not to checkout or execute untrusted code from the fork in a way that could lead to security vulnerabilities (such as command injection) [3]. Always ensure the workflow only performs safe, expected operations. If these methods do not meet your needs, or if you require permissions beyond what the GITHUB_TOKEN provides, you can use a Personal Access Token (PAT) or a GitHub App installation access token, stored as a repository secret, to perform authenticated API requests [7][8][3].

Citations:


Restrict the workflow token and disable checkout credential persistence.

This PR-triggered workflow uses actions/checkout@v4 without least-privilege permissions, and persist-credentials defaults to true, leaving the authentication token available to later steps via local Git configuration. Set permissions: contents: read and persist-credentials: false since the workflow only reads and tests repository code.

Proposed hardening
 jobs:
     test:
+        permissions:
+            contents: read
         runs-on: ubuntu-latest

         steps:
-            - uses: actions/checkout@v4
+            - uses: actions/checkout@v4
+              with:
+                persist-credentials: false
📝 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
jobs:
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
jobs:
test:
permissions:
contents: read
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
persist-credentials: false
🧰 Tools
🪛 zizmor (1.26.1)

[warning] 14-14: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 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 @.github/workflows/ci.yml around lines 9 - 14, Harden the test job by
declaring read-only repository permissions with contents: read and configure
actions/checkout@v4 with persist-credentials disabled. Apply these changes to
the workflow-level or job-level permissions and the checkout step, preserving
the existing test steps.

Source: Linters/SAST tools

This was referenced Aug 4, 2026
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