From 5562325702edb8f1e1b15c8641e8839b563738a6 Mon Sep 17 00:00:00 2001 From: heznpc Date: Thu, 21 May 2026 21:55:11 +0900 Subject: [PATCH] =?UTF-8?q?fix:=202nd-pass=20audit=20=E2=80=94=20previousl?= =?UTF-8?q?y=20declared=20done,=20second-pass=20caught?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial second-pass on commits 01ed3052 (PR #26) and ec7045df (PR #27), both reported as "done / tests pass / CI green". Two Critical findings. [C1] CodeQL `analyze (actions)` open alert - Rule: `actions/missing-workflow-permissions` - File: .github/workflows/cd.yml - Message: "Actions job or workflow does not limit the permissions of the GITHUB_TOKEN." - Detected by the CodeQL `actions` language matrix that PR #26 itself added. PR #26 introduced top-level permissions on most workflows but missed cd.yml and setup.yml. The 5 required CI checks didn't include CodeQL, so the alert landed silently after merge. - Fix: add `permissions: contents: read` at workflow top level in cd.yml and setup.yml. Jobs continue to opt into the writes they need (contents:write / id-token:write / attestations:write for publish; issues:write for setup). [C2] `ok()` and `err()` Response Helpers — dead code marketed as feature - server.py:41 and server.py:47 defined ok() / err() helpers. - Zero references anywhere in src/ or tests/ (`grep -rn "\\bok(\\|\\berr("` = only the definitions + the introductory comment). - Coverage report confirmed: server.py:43-44, 49 uncovered. - README.md:32 and README.ko.md:30 marketed these as "Response Helpers — ok() and err() for consistent tool responses" in the "What You Get" list — i.e. a public feature claim with no backing code usage. - The bundled `greet` tool returns plain `str` — the README's marketed pattern doesn't match the bundled example. - FastMCP's idiom is `return value` / `raise on error`; ok/err helpers are a raw-MCP-without-FastMCP relic that actively teach the wrong pattern. - Fix: delete ok() / err() / their introductory comment. Replace the Tools-section banner comment with one line documenting the actual FastMCP idiom. Remove the "Response Helpers" bullet from README.md and README.ko.md. Why this matters beyond cosmetics: a starter is a teaching artifact. False "What You Get" entries silently propagate the wrong pattern to every clone. The dead helpers were also reachable code surface that CodeQL has to scan but no test exercises. Verification - All 7 workflows now have top-level `permissions:` (verified via `awk` scan of column-1 `permissions:` line). - ruff / ruff format / mypy strict / pytest 17/17 all clean. - Coverage 72.12% (improved from 70.64% — uncovered dead code removed). Out-of-scope for this PR (will be raised as Major findings for user decision): - tests/test_tools.py has no `test_greet_registered_on_server` despite test_server_info.py and test_code_review.py both having that pattern. - server_info.py wheel-install fallback path (lines 41-42, 52-56) is 0% covered; test only exercises the dev/source-tree read-pyproject path. - src/my_mcp_server/tools/greet.py defines a `register()` that's never called by server.py — fully dead modular example. - SECURITY.md "branch protection on main" claim is starter-side only; clones inherit nothing. - 4 Dependabot PRs (#28, #29, #30, #31) opened today — #28 is attest-build-provenance v3.2.0 → v4.1.0, making our 6-hour-old SHA pin already one major behind. --- .github/workflows/cd.yml | 5 +++++ .github/workflows/setup.yml | 4 ++++ README.ko.md | 1 - README.md | 1 - src/my_mcp_server/server.py | 19 ++----------------- 5 files changed, 11 insertions(+), 19 deletions(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index c7f2b61..4a6f346 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -3,6 +3,11 @@ name: Publish to PyPI on: workflow_dispatch: +# Deny-by-default at workflow level; each job opts into exactly what it needs. +# Flagged by CodeQL `actions/missing-workflow-permissions`. +permissions: + contents: read + concurrency: group: deploy cancel-in-progress: false diff --git a/.github/workflows/setup.yml b/.github/workflows/setup.yml index 31e213e..c704018 100644 --- a/.github/workflows/setup.yml +++ b/.github/workflows/setup.yml @@ -4,6 +4,10 @@ on: push: branches: [main] +# Deny-by-default at workflow level; the `setup` job opts into `issues: write` only. +permissions: + contents: read + jobs: setup: if: github.run_number == 1 diff --git a/README.ko.md b/README.ko.md index 9452354..e8c7691 100644 --- a/README.ko.md +++ b/README.ko.md @@ -27,7 +27,6 @@ MCP 서버를 만들고, 원클릭 배포. 시크릿 불필요. - **MCP 3대 프리미티브** — Tools, Resources, Prompts 예제 전부 포함 - **Safety Annotations** — 모든 도구에 readOnly/destructive/idempotent 힌트 - **검증된 Prompt** — pydantic `@validate_call`로 핸들러 실행 전 인자 검증 -- **응답 헬퍼** — `ok()`, `err()`로 일관된 응답 - **CI** — gitleaks, ruff, 라이선스 검증, pytest (3.11/3.12/3.13) - **CD** — OIDC trusted publishing으로 PyPI 배포 (시크릿 불필요) - **Dependabot** — 의존성 + GitHub Actions 자동 업데이트 diff --git a/README.md b/README.md index 2d1083c..6592ddf 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,6 @@ Build your MCP server. One-click publish. Zero secrets needed. - **All three MCP primitives** — Tools, Resources, and Prompts with working examples - **Safety Annotations** — readOnly/destructive/idempotent hints on every tool - **Validated Prompts** — pydantic `@validate_call` rejects bad args before the handler runs -- **Response Helpers** — `ok()` and `err()` for consistent tool responses - **Config** — Environment variable parsing pattern - **CI** — gitleaks, ruff, license compliance, pytest (3.11/3.12/3.13) - **CD** — OIDC trusted publishing to PyPI (zero secrets needed) diff --git a/src/my_mcp_server/server.py b/src/my_mcp_server/server.py index 178ac2c..3735b9d 100644 --- a/src/my_mcp_server/server.py +++ b/src/my_mcp_server/server.py @@ -34,23 +34,8 @@ # --------------------------------------------------------------------------- -# Helpers — use ok() and err() for consistent tool responses -# --------------------------------------------------------------------------- - - -def ok(data: str | dict[str, object]) -> dict[str, object]: - """Return a successful tool response.""" - text = data if isinstance(data, str) else str(data) - return {"content": [{"type": "text", "text": text}]} - - -def err(message: str) -> dict[str, object]: - """Return an error tool response.""" - return {"content": [{"type": "text", "text": message}], "isError": True} - - -# --------------------------------------------------------------------------- -# Tools — add your own in tools/ and import here +# Tools — FastMCP wraps return values automatically (return value for success, +# raise for errors). Add your own in tools/ and import here. # ---------------------------------------------------------------------------