diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index f59c1f3..b7c4330 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -20,6 +20,21 @@ --- +## Security-sensitive change checklist + + + +- [ ] Ran `/security-review` (note: by design it does not check for denial-of-service or resource-exhaustion issues — those need the next steps) +- [ ] Considered adversarial/misuse scenarios and checked the code against each (list them below) +- [ ] Added a regression test that fails against the pre-fix code for anything found this way +- [ ] Checked resource bounds: timeouts, cache eviction/max size, request/entry limits + +**Scenarios considered:** + + + +--- + ## Additional Notes diff --git a/docs/engineering_standards.md b/docs/engineering_standards.md index 64a34ae..701f625 100644 --- a/docs/engineering_standards.md +++ b/docs/engineering_standards.md @@ -122,6 +122,17 @@ Observability is opt-in via OpenTelemetry. Enable only when debugging: - Use Jaeger for local development: `npm run tracing:jaeger:start` - See `docs/tracing.md` for detailed configuration +**Security-Sensitive Change Review:** + +For changes that touch authentication, credentials, tokens, or session/connection state where an external user is involved, review needs more than a diff read: + +1. Run the `/security-review` skill. It's good at authentication, authorization, injection, and crypto issues, but its own instructions explicitly exclude denial-of-service and resource-exhaustion findings, so a clean result doesn't mean those are covered. +2. Write down 2-3 concrete adversarial or misuse scenarios for the change (e.g. "what if this instance is shared across two concurrent sessions", "what if a client returns a value far larger than expected", "what if this token isn't verified the way we assume") and check the code against each one. +3. Add a regression test that fails against the pre-fix code for anything found this way — proof the issue was real, not just "looks fixed." +4. Get 2+ reviewer approvals as a floor, treated as a backstop rather than the primary defense. + +This doesn't apply to every PR, only ones where getting it wrong could expose one user's data or credentials to another, or let a shared resource be exhausted by external input. PR #57 in this repo is a 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, and were only caught by later, independently-run adversarial passes. + **Docker:** The project includes Docker support for containerized deployment: