$npx -y skills add warpdotdev/oz-for-oss --skill security-review-prAudit a pull request diff for common security concerns (input validation, sanitization, authentication and authorization, secrets management, unsafe dependencies, and related risks) and fold findings into the same review.json produced by the base PR review. Use as a supplement to
| 1 | # Security Review PR Skill |
| 2 | |
| 3 | Audit the current pull request for security concerns and fold any findings into the same `review.json` produced by the base `review-pr` skill. |
| 4 | |
| 5 | ## Goal |
| 6 | |
| 7 | Provide a focused security pass on top of the general PR review. This is a supplement to `review-pr`, not a separate output. Findings must be merged into the single combined `review.json` so reviewers receive one cohesive review. |
| 8 | |
| 9 | ## Inputs |
| 10 | |
| 11 | - The working directory is the PR branch checkout. |
| 12 | - The workflow usually provides an annotated diff in `pr_diff.txt`. |
| 13 | - The workflow usually provides the PR description in `pr_description.md`. |
| 14 | - Focus on the files and lines changed by this PR. |
| 15 | - Default behavior: do not post comments or reviews to GitHub directly. |
| 16 | |
| 17 | ## When to apply this skill |
| 18 | |
| 19 | - Apply on code PRs whenever `review-pr` is applied. |
| 20 | - Do not apply on spec-only PRs handled by `review-spec`. |
| 21 | - Skip the skill entirely when no changed file introduces code or configuration that touches the concerns below; it is better to stay silent than to manufacture findings. |
| 22 | - Do not duplicate findings the base `review-pr` pass will already raise. If the base review would naturally catch an issue, leave it there rather than re-reporting it from the security pass. |
| 23 | |
| 24 | ## Security concerns to audit |
| 25 | |
| 26 | Evaluate each changed hunk against the following concerns. Treat the list as a checklist, not a ceiling — flag other clearly security-relevant issues when they appear. |
| 27 | |
| 28 | ### Input validation and untrusted data |
| 29 | - User-supplied or network-supplied input used without validation, length limits, or type checks. |
| 30 | - Deserialization of untrusted data (e.g. `pickle`, `yaml.load`, `eval`, `Function`, `JSON.parse` feeding into a command). |
| 31 | - Path traversal risk: user input concatenated into filesystem paths without normalization or an allowlist. |
| 32 | - SSRF risk: user-controlled URLs passed to outbound HTTP clients without scheme or host restrictions. |
| 33 | - Unbounded resource use driven by untrusted input (memory, loops, regex with catastrophic backtracking). |
| 34 | |
| 35 | ### Output encoding and sanitization |
| 36 | - Untrusted data interpolated into SQL, shell commands, HTML, Markdown, log lines, or URLs without the right encoding or parameterization. |
| 37 | - Use of `shell=True`, string concatenation into `exec`/`spawn`, or raw SQL strings. |
| 38 | - Rendering user-supplied Markdown or HTML into a UI without sanitization. |
| 39 | |
| 40 | ### Authentication and authorization |
| 41 | - Missing or weakened authentication checks on new endpoints, RPC handlers, or CLI commands. |
| 42 | - Authorization checks that trust client-supplied identifiers instead of the authenticated principal. |
| 43 | - Permission checks removed or made more permissive without clear justification. |
| 44 | |
| 45 | ### Secrets management |
| 46 | - Hardcoded credentials, API keys, private keys, or tokens committed in source. |
| 47 | - Secrets read from insecure locations (world-readable files, logs, environment dumps printed to stdout). |
| 48 | - Secrets passed via command-line arguments where they would leak into process listings or shell history. |
| 49 | - Secrets written to logs, error messages, analytics events, or serialized payloads. |
| 50 | - New secrets added to `.env`, fixtures, or test data that are real rather than clearly synthetic placeholders. |
| 51 | |
| 52 | ### Cryptography and randomness |
| 53 | - Use of weak or deprecated primitives (MD5, SHA1 for security purposes, ECB mode, RC4). |
| 54 | - Hand-rolled crypto when a vetted library is available. |
| 55 | - Non-cryptographic randomness (`random.random`, `Math.random`) used for tokens, IDs, or security decisions. |
| 56 | - Missing integrity or authenticity checks when decrypting or verifying tokens. |
| 57 | |
| 58 | ### Dependencies and supply chain |
| 59 | - New dependencies from unknown registries or forks without a clear rationale. |
| 60 | - Pinning loosened in a way that allows untrusted upgrades (e.g. `*` or very broad ranges on a sensitive package). |
| 61 | - Fetching scripts over HTTP or piping curl to a shell inside build or CI steps. |
| 62 | |
| 63 | ### Data handling and privacy |
| 64 | - Logging or echoing personally identifiable information, auth tokens, session IDs, or request bodies that may contain them. |
| 65 | - New telemetry or analytics events that capture sensitive data without redaction. |
| 66 | - Expanding the scope of stored data beyond what the feature requires. |
| 67 | |
| 68 | ### Configuration and defaults |
| 69 | - New feature flags or configuration options that default to insecure values (e.g. TLS disabled, auth optional). |
| 70 | - CORS, cookies, or headers weakened in scope without an explicit justification. |
| 71 | - Permissive file modes on newly created files that contain sensitive material. |
| 72 | |
| 73 | ## Process |
| 74 | |
| 75 | 1. Read `pr_description.md` and `pr_diff.txt` to understand the scope and intent of the change. |
| 76 | 2. For each changed hunk, consid |