microsoft / microsoft/hve-core
[security] Audit MCP server entrypoints under .github/skills/** for credential exposure
- Dominant language
- Python
- Stars
- 1.5k
- Forks
- 301
- Avg merge
- 3d 3h
- Merged PRs (30d)
- 92
Description
## Background
The Mural skill is the only true MCP (Model Context Protocol) server
entrypoint shipped in this repo today. It runs a stdio framing loop using
`_frame_mcp_message` / `_parse_mcp_frame` reading from `sys.stdin.buffer`
(`.github/skills/mural/mural/scripts/mural.py` L11486 region) — this is the
JSON-RPC framing surface that downstream MCP clients connect to.
By contrast, the Jira and GitLab skills' `sys.stdin.read()` calls are for
**CLI JSON-payload arguments** (e.g. `handle_comment` reading a comment
body from stdin — `.github/skills/jira/jira/scripts/jira.py` L323), **not**
MCP framing. They are not MCP servers.
This distinction matters because:
1. **MCP servers process untrusted client framing.** Their stdio path is a
trust boundary that must redact credentials in any frame echoed back,
any error frame written to stderr, and any traceback raised through the
framing loop.
2. **CLI stdin is a trust boundary too**, but a different one — the caller
is the operator running the script with controlled arguments.
3. The repo currently has **no inventory** of MCP server entrypoints and
**no policy** requiring new MCP server skills to adopt the redaction
triad before merge.
This issue closes that gap before another team ships a new MCP server skill
without a redaction layer.
## Required changes
### 1. MCP server inventory document
Create `docs/security/mcp-server-inventory.md` documenting every MCP server
entrypoint pattern under `.github/skills/**`:
- Current state: mural is the sole MCP server entrypoint.
- File: `.github/skills/mural/mural/scripts/mural.py`
- Framing functions: `_frame_mcp_message`, `_parse_mcp_frame`
- Stdio surface: `sys.stdin.buffer`
- Trust boundary documented in
`.github/skills/mural/mural/SECURITY.md` Bucket B4.
- Detection patterns (regex / grep) used to identify MCP server code:
- `_frame_mcp_message`
- `_parse_mcp_frame`
- `sys.stdin.buffer.read`
- `Content-Length:` (HTTP-style framing header used by JSON-RPC over
stdio per the MCP spec)
- JSON-RPC framing loop signatures.
### 2. Policy section
The inventory document must contain a *Policy* section stating:
> Any new MCP server entrypoint added under `.github/skills/**` MUST adopt
> the redaction triad documented in
> `.github/skills/mural/mural/SECURITY.md` Bucket B4 before merge:
>
> - `_REDACT_KEYS` tuple covering all credential field names accepted by
> the server.
> - `_REDACT_PATTERNS` covering Authorization headers, JSON shapes, form
> shapes, and any service-specific token query parameters.
> - `_redact()` helper applied to every framed message logged or echoed.
> - `_emit()` central output sink wrapping all stdout/stderr writes.
> - `_emit_debug_traceback()` gated by an explicit debug env var, with
> `_redact()` applied to every traceback frame.
> - Typed `*APIError` class with a `__str__` override that excludes raw
> upstream body content.
> - Per-skill `tests/test_redaction.py` source-contract tests that assert
> the framing loop only logs through `_redact()` (mirror
> `test_logger_token_post_wraps_url_in_redact` in mural).
### 3. CI lint
Add a CI lint that flags any new file matching the MCP server detection
patterns (above) outside `.github/skills/mural/**` without an associated
`_redact` adoption checklist commit. Implementation options:
- **Option A**: Extend `npm run lint:permissions` (existing PowerShell-
driven lint) to grep for the framing patterns and require a sentinel
comment `# MCP-REDACT-OK` near the framing entrypoint.
- **Option B**: Add a new `npm run lint:mcp-policy` script under
`scripts/security/` that performs the same grep + checklist check and
fails CI when a new MCP server entrypoint lands without sentinel.
The script must:
- Recursively scan `.github/skills/**/scripts/*.py`.
- Match the regex set defined in §1 (Detection patterns).
- For each match outside `.github/skills/mural/**`, fail unless the file
contains the `# MCP-REDACT-OK` sentinel **and** a sibling
`tests/test_redaction.py` exists.
- Output JSON to `logs/mcp-policy-results.json` per repo convention.
### 4. Deeper grep audit (run during this issue, record results)
To confirm zero false negatives with the current state, run during
implementation:
```bash
grep -rnE '(sys\.stdin\.buffer\.read|_parse_mcp_frame|_frame_mcp_message|Content-Length:.*\\r\\n)' \
.github/skills/**/scripts/*.py
```
Record the result set in the inventory document. Expected baseline: only
matches inside `.github/skills/mural/mural/scripts/mural.py`.
### 5. Operator credential audit
For each entry confirmed as an MCP server entrypoint, audit:
- Which env vars carry credentials (e.g. `MURAL_*`).
- Whether each credential appears in `_REDACT_KEYS` or `_REDACT_PATTERNS`.
- Whether the inventory document reflects that mapping.
## Acceptance criteria
- [ ] `docs/security/mcp-server-inventory.md` exists and lists mural as
the sole entrypoint.
- [ ] Policy section is present and references mural Bucket B4 as the
template.
- [ ] Detection patterns documented with the regex set.
- [ ] CI lint configured (Option A or Option B above) and wired into
`npm run lint:all`.
- [ ] CI lint output JSON written to `logs/`.
- [ ] Grep audit results recorded in the inventory document.
- [ ] Operator credential audit table present for mural and any future
entrypoints.
- [ ] `markdownlint` clean on the inventory document.
- [ ] `npm run lint:md` clean.
## Dependencies
- Independent of the redaction-port and CI-gating issues; can land in
parallel.
- Should land **before** any new MCP server skill PR is merged, otherwise
the policy is bypassed.
## References
- `.github/skills/mural/mural/scripts/mural.py` — `_frame_mcp_message`,
`_parse_mcp_frame`, `sys.stdin.buffer` (L11486 region)
- `.github/skills/mural/mural/SECURITY.md` Bucket B4 (policy template)
- `.github/skills/jira/jira/scripts/jira.py` L323 (NOT MCP — CLI stdin
payload only)
- `.github/skills/gitlab/gitlab/scripts/gitlab.py` (NO stdin reads)
- `scripts/security/` (existing security lint pattern)
- `scripts/linting/` (existing lint pattern + JSON output convention)
- `package.json` `lint:all` chain
Contributor guide
Assessment
This issue has not been assessed yet.