Add security review template and checklist #63

ericelliott · 2025-12-27T01:57:06Z

No description provided.

Copilot

Pull request overview

This PR introduces a comprehensive security review template and checklist to guide security audits of authentication flows, session management, and mutating routes. The template provides structured guidance for conducting OWASP Top 10 vulnerability scans and other security assessments.

Adds a markdown template for generating security review reports with executive summaries, quality scores, and findings
Provides detailed security checklists covering authentication, OWASP Top 10 vulnerabilities, JWT security, and timing-safe comparisons
Defines a /security command for generating comprehensive security reports

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-27T01:59:24Z

ai/commands/security.md

+Documentation quality: ${ score }/10
+Test coverage and quality: ${ score }/10
+Critical findings: ${ count }
+High severity findings: ${ count }
+Medium severity findings: ${ count }
+Low severity findings: ${ count }
+
+## Summary of Findings
+
+$summaryOfFindings


The template uses different placeholder syntaxes inconsistently throughout. Lines 26-31 use "${ score }" and "${ count }" with curly braces and spaces, while lines 16-19 use "$date", "$repoUrl", "$commitHash" without braces, and line 35 uses "$summaryOfFindings". This inconsistency could lead to confusion when implementing the template. Consider standardizing on a single placeholder format throughout the template.

Copilot · 2025-12-27T01:59:24Z

ai/commands/security.md

+${ for each file }:
+### $fileName
+
+${ for each checklistItem }
+${ ✅ | ❌ | ⚠️ } $ItemName
+
+${ for each issue }
+**${issue}:** ${issueDescription}
+
+${ codeSnippet }
+
+${ briefExplanation }


The template includes pseudo-code control flow syntax "${ for each file }" and "${ for each checklistItem }" (lines 39, 42, 45) that doesn't match any standard templating language. This pseudo-syntax might confuse implementers about what templating engine to use or how to implement these loops. Consider either using a specific templating language syntax (like Handlebars, Mustache, or Jinja2) or clarifying in comments that this is pseudo-code that needs to be replaced with actual templating logic.

ai/commands/security.md

Copilot · 2025-12-27T01:59:25Z

ai/commands/security.md

+
+## Executive Summary
+
+This review was propared by by Parallel Drive - world class security trusted by apps that secure billions in value.


Spelling error: There's a duplicate "by" in the text. It reads "propared by by Parallel Drive" but should only have one "by".

Suggested change

This review was propared by by Parallel Drive - world class security trusted by apps that secure billions in value.

This review was propared by Parallel Drive - world class security trusted by apps that secure billions in value.

Copilot · 2025-12-27T01:59:25Z

ai/commands/security.md

+- Timing-Safe Compare (use hashing)
+  - Never compare raw secrets with equality or `crypto.timingSafeEqual`; avoid XOR/accumulation tricks.
+  - Always hash both stored secret and candidate using SHA3 and compare hashes.
+  - Applies to ALL security token compares (webhooks, reset tokens, CSRF, API keys, signatures).
+  - If "timing safe compare" on raw value detected => CRITICAL bug report; include rationale in code comment to prevent regressions.


The advice to "Never compare raw secrets with equality or crypto.timingSafeEqual" is misleading. The crypto.timingSafeEqual function is specifically designed for secure constant-time comparison of secrets and is a legitimate security best practice. While the advice to hash values before comparison is valid in certain contexts, completely discouraging the use of crypto.timingSafeEqual could lead developers to avoid a properly designed security function. Consider clarifying that crypto.timingSafeEqual is acceptable for certain use cases, or explaining more precisely when hashing is preferred over direct timing-safe comparison.

Suggested change

- Timing-Safe Compare (use hashing)

- Never compare raw secrets with equality or `crypto.timingSafeEqual`; avoid XOR/accumulation tricks.

- Always hash both stored secret and candidate using SHA3 and compare hashes.

- Applies to ALL security token compares (webhooks, reset tokens, CSRF, API keys, signatures).

- If "timing safe compare" on raw value detected => CRITICAL bug report; include rationale in code comment to prevent regressions.

- Timing-Safe Compare (hashing and constant-time)

- Never compare secrets or secret-derived values (tokens, MACs, hashes) with plain equality (`==`/`===`); avoid XOR/accumulation or other DIY constant-time tricks.

- Use a well-reviewed constant-time compare (for example, Node.js `crypto.timingSafeEqual` on equal-length `Buffer`s) when you must compare raw secrets or fixed-length secret-derived values.

- Prefer storing only secret hashes or MACs (for example, using SHA-3/SHA-256, HMAC, or password hashing schemes such as bcrypt/Argon2), and compare those values using a constant-time compare.

- Applies to ALL security token compares (webhooks, reset tokens, CSRF, API keys, signatures).

- If a non-constant-time equality check on secrets or secret-derived values is detected => CRITICAL bug report; include rationale in code comment to prevent regressions.

ai/commands/security.md

Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-05T02:48:39Z

ai/commands/security.md

+Date: $date
+Expires: ${ date + 90 days }
+Source repository: $repoUrl
+Commit hash: $commitHash
+
+> ## Disclaimer
+> This review scope is limited to the codebase at the specific commit hashes listed on the front page. Any features or changes introduced after these commits are excluded from this report.
+
+## Quality Scores
+
+Documentation quality: ${ score }/10
+Test coverage and quality: ${ score }/10
+Critical findings: ${ count }
+High severity findings: ${ count }
+Medium severity findings: ${ count }
+Low severity findings: ${ count }
+
+## Summary of Findings
+
+$summaryOfFindings
+
+## $route
+
+${ for each file }:
+### $fileName
+
+${ for each checklistItem }
+${ ✅ | ❌ | ⚠️ } $ItemName
+
+${ for each issue }
+**${issue}:** ${issueDescription}


Inconsistent placeholder syntax throughout the template. Lines use different formats: $date, ${score}, $summaryOfFindings, $route, $fileName, etc. For consistency and clarity in documentation templates, standardize on one format (either $variable or ${variable}).

Copilot · 2026-01-05T02:48:40Z

ai/commands/security.md

+${ for each file }:
+### $fileName
+
+${ for each checklistItem }
+${ ✅ | ❌ | ⚠️ } $ItemName
+
+${ for each issue }


The loop syntax ${ for each file }: and ${ for each checklistItem } is not valid JavaScript or any standard templating language. This pseudocode should be clarified with a comment indicating it's illustrative, or replaced with actual template syntax if this is meant to be executable. Consider using clearer documentation notation like [for each file in route] or add a note explaining this is pseudocode.

Suggested change

${ for each file }:

### $fileName

${ for each checklistItem }

${ ✅ | ❌ | ⚠️ } $ItemName

${ for each issue }

[for each file in route]:

### $fileName

[for each checklist item]:

${ ✅ | ❌ | ⚠️ } $ItemName

[for each issue]:

Copilot · 2026-01-05T02:48:40Z

ai/commands/security.md

+
+Output template: 
+
+Filename: `$projectRoot/security/reports/${yyyy-mm-dd}-security-foundations-report.md`


The filename template uses ${yyyy-mm-dd} which is not a valid JavaScript date format expression. This should be clarified as a placeholder format. Consider using a clearer notation like [YYYY-MM-DD] or {date} to indicate this should be replaced with an actual date string in that format.

Suggested change

Filename: `$projectRoot/security/reports/${yyyy-mm-dd}-security-foundations-report.md`

Filename: `$projectRoot/security/reports/[YYYY-MM-DD]-security-foundations-report.md`

Copilot · 2026-01-05T02:48:40Z

ai/commands/security.md

+"""
+# $projectName Security Review
+
+## Executive Summary
+
+This review was prepared by Parallel Drive - world class security trusted by apps that secure billions in value.
+
+Date: $date
+Expires: ${ date + 90 days }
+Source repository: $repoUrl
+Commit hash: $commitHash
+
+> ## Disclaimer
+> This review scope is limited to the codebase at the specific commit hashes listed on the front page. Any features or changes introduced after these commits are excluded from this report.
+
+## Quality Scores
+
+Documentation quality: ${ score }/10
+Test coverage and quality: ${ score }/10
+Critical findings: ${ count }
+High severity findings: ${ count }
+Medium severity findings: ${ count }
+Low severity findings: ${ count }
+
+## Summary of Findings
+
+$summaryOfFindings
+
+## $route
+
+${ for each file }:
+### $fileName
+
+${ for each checklistItem }
+${ ✅ | ❌ | ⚠️ } $ItemName
+
+${ for each issue }
+**${issue}:** ${issueDescription}
+
+${ codeSnippet }
+
+${ briefExplanation }
+"""


The closing triple quotes on line 51 should use backticks to match the opening on line 9 if this is meant to represent a Markdown code block or template. Currently line 9 uses """ and line 51 also uses """, which is not standard Markdown syntax. Consider using triple backticks (```) for consistency with Markdown conventions.

Copilot · 2026-01-05T02:48:41Z

ai/commands/security.md

+  - Verify JWT session strategy config
+  - Validate cookie settings via `defaultCookies(https)` for `Secure`, `HttpOnly`, `SameSite`, domain/path consistency.
+  - Confirm CSRF protections on auth endpoints (e.g. double-submit cookie method; `/api/auth/csrf` when applicable).
+  - Verify 2FA/TOTP and backup code handling (encryption at rest, single-use, compare hashes to mitigate hangman and sidestep timing safe compare pitfalls, deletion upon use).


The phrase "hangman and sidestep timing safe compare pitfalls" is unclear and grammatically awkward. Consider rephrasing for clarity, such as "mitigate timing attacks and avoid timing-safe comparison pitfalls" or "prevent timing attacks through proper hashing".

Suggested change

- Verify 2FA/TOTP and backup code handling (encryption at rest, single-use, compare hashes to mitigate hangman and sidestep timing safe compare pitfalls, deletion upon use).

- Verify 2FA/TOTP and backup code handling (encryption at rest, single-use, compare hashes to mitigate timing attacks and avoid timing-safe comparison pitfalls, deletion upon use).

Copilot · 2026-01-05T02:48:41Z

ai/commands/security.md

+  - Assess input validation with Zod/DTO schemas; sanitize external inputs.
+  - Validate webhook security (signatures, replay protection, IP allowlists where relevant).
+- JWT Security Review
+  If JWT use is detected, for each jwt-flow { carefully run all checks in ai/rules/security/jwt-security.mdc one at a time. }


The loop syntax for each jwt-flow { carefully run all checks in ai/rules/security/jwt-security.mdc one at a time. } uses inconsistent brace notation and unclear pseudocode. This should be clarified as instructional text rather than attempting to look like code syntax. Consider rephrasing to: "For each JWT flow detected, carefully run all checks in ai/rules/security/jwt-security.mdc one at a time."

Suggested change

If JWT use is detected, for each jwt-flow { carefully run all checks in ai/rules/security/jwt-security.mdc one at a time. }

If JWT use is detected, carefully run all checks in `ai/rules/security/jwt-security.mdc` for each JWT flow detected, one at a time.

Add security review template and checklist

c2b8322

Copilot AI review requested due to automatic review settings December 27, 2025 01:57

Copilot started reviewing on behalf of ericelliott December 27, 2025 01:57 View session

Copilot AI reviewed Dec 27, 2025

View reviewed changes

Apply suggestion from @Copilot

1cab62c

Co-authored-by: Copilot <[email protected]>

Copilot AI review requested due to automatic review settings January 5, 2026 02:46

Copilot started reviewing on behalf of ericelliott January 5, 2026 02:46 View session

Copilot AI reviewed Jan 5, 2026

View reviewed changes


		## Executive Summary

		This review was propared by by Parallel Drive - world class security trusted by apps that secure billions in value.

	This review was propared by by Parallel Drive - world class security trusted by apps that secure billions in value.
	This review was propared by Parallel Drive - world class security trusted by apps that secure billions in value.


		Output template:

		Filename: `$projectRoot/security/reports/${yyyy-mm-dd}-security-foundations-report.md`

	- Verify 2FA/TOTP and backup code handling (encryption at rest, single-use, compare hashes to mitigate hangman and sidestep timing safe compare pitfalls, deletion upon use).
	- Verify 2FA/TOTP and backup code handling (encryption at rest, single-use, compare hashes to mitigate timing attacks and avoid timing-safe comparison pitfalls, deletion upon use).

	If JWT use is detected, for each jwt-flow { carefully run all checks in ai/rules/security/jwt-security.mdc one at a time. }
	If JWT use is detected, carefully run all checks in `ai/rules/security/jwt-security.mdc` for each JWT flow detected, one at a time.

Add security review template and checklist #63

Are you sure you want to change the base?

Add security review template and checklist #63

Uh oh!

Conversation

ericelliott commented Dec 27, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants