close
Skip to content

Add security-sensitive change review checklist - #42

Open
mattpodwysocki wants to merge 1 commit into
mainfrom
docs/security-sensitive-change-review
Open

Add security-sensitive change review checklist#42
mattpodwysocki wants to merge 1 commit into
mainfrom
docs/security-sensitive-change-review

Conversation

@mattpodwysocki

Copy link
Copy Markdown
Contributor

Summary

  • Prompted by a Slack discussion (Valentin) about relying on reviewer intuition/good intentions to catch high-blast-radius issues (auth, credentials, session state) — proposed requiring 2+ peer reviews for such changes.
  • mapbox/mcp-devkit-server#57 is the worked example we used: a cross-session credential-hijack bug and several resource-exhaustion gaps got past the original implementation and a full round of human review, and were only caught by later, independently-run adversarial passes.
  • Adds a visible checklist to the PR template (fill in only when the change touches auth/credentials/tokens/session state and an external user is involved) plus matching guidance in CONTRIBUTING.md.
  • This is a visible prompt, not a CI gate — same approach being rolled out across mcp-server, mcp-devkit-server, mcp-docs-server, and hosted-mcp-server.

Test plan

  • Docs-only change; no code paths affected
  • Reviewer confirms the new PR template section renders correctly on a fresh PR

Prompted by a Slack discussion about relying on reviewer intuition to
catch high-blast-radius issues (auth, credentials, session state).
mcp-devkit-server PR #57 is the worked example: a cross-session
credential-hijack bug and several resource-exhaustion gaps got past
the original implementation and a full round of human review, caught
only by later, independently-run adversarial passes.

Adds a visible PR template section plus CONTRIBUTING guidance covering:
running /security-review (noting its explicit DoS/resource-exhaustion
exclusions), writing down adversarial scenarios, adding a regression
test proving the bug was real, and 2+ reviewer approval as a backstop.
This is a prompt, not a CI gate.
@mattpodwysocki
mattpodwysocki requested a review from a team as a code owner August 14, 2026 14:08
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