feat: add @droid security commands #17
Conversation
![factory-droid[bot]](https://avatars.githubusercontent.com/in/358017?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overall the security-command additions look directionally correct, but there are a couple of input/flow edge cases that can lead to non-deterministic behavior (NaN scan days) or prompts missing the tracking comment id in some paths. Addressing the inline notes should make the new security flows more robust.
There was a problem hiding this comment.
Overall: the feature wiring looks coherent, but the security-review prompt currently instructs potentially unsafe/incorrect behavior (committing files to the contributor’s PR and suppressing inline comments), which can break expected review semantics. Fixing those prompt directives should make the new security flows safer and more reliable.
|
Droid finished @shashank-factory's task —— View job |
Add support for security-focused code review commands: - @droid security - Security review on PR changes - @droid security --full - Full repository security scan New features: - Security command parser (security, review-security, security-full) - Security review prompt with STRIDE methodology - Security scan prompt with threat model generation - Security configuration inputs in action.yml - Security-specific tracking comment message Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Fix SECURITY_SCAN_DAYS to avoid NaN (clamp to positive integer, default 7) - Remove instructions to commit threat model to PR branch during review - Remove instructions to commit patches to PR branch - Align security review with JSON output pattern (no direct inline comments) Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
| ## Output Requirements | ||
| IMPORTANT: Do NOT post inline comments directly. Instead, write findings to a JSON file. | ||
| The finalize step will post all inline comments to avoid overlapping with code review comments. |
There was a problem hiding this comment.
[P1] Don’t forbid inline comments without providing a poster
This prompt says “do NOT post inline comments directly” and instead write security-review-results.json, but there’s no code in this PR that reads that file and posts the comments—so security reviews can silently produce zero inline feedback.
varin-nair-factory
force-pushed
the
ssharma/pr2-security-commands
branch
from
dff43cf to
01e9df8
Compare
There was a problem hiding this comment.
Overall: the PR is mostly correct and prior P1 issues look addressed, but the security-review prompt still instructs writing a JSON file instead of posting inline comments, and there’s no mechanism added here to publish those findings—so security review runs can end up with no inline feedback.
| ); | ||
| if (combinedMatch) { | ||
| return { | ||
| command: "review-security", |
There was a problem hiding this comment.
what is review-security? Is that where both code review and security review get triggered?
| const command = extractCommandFromContext(context); | ||
| if (command && ['fill', 'review'].includes(command.command)) { | ||
| console.log(`Detected @droid ${command.command} command, triggering action`); | ||
| if (command && ["fill", "review", "security"].includes(command.command)) { |
There was a problem hiding this comment.
Shouldn't we also check for security-full command?
| const securityModel = | ||
| process.env.SECURITY_MODEL?.trim() || process.env.REVIEW_MODEL?.trim(); |
There was a problem hiding this comment.
The code prefers SECURITY_MODEL over REVIEW_MODEL as a fallback, which is sensible. However, this precedence isn't documented in action.yml descriptions, which could confuse users who set both variables.
| const isSecurityCommand = | ||
| context.inputs.automaticSecurityReview || | ||
| commandContext?.command === "security" || | ||
| commandContext?.command === "security-full"; |
There was a problem hiding this comment.
what about security-review? You added that as an option in DroidCommand (command-parser.ts)
varin-nair-factory
left a comment
varin-nair-factory
left a comment
There was a problem hiding this comment.
approving to unblock, but pls see comments
Summary
Add support for security-focused code review commands.
New Commands
@droid security- Security review on PR changes using STRIDE methodology@droid security --full- Full repository security scan with threat model generationChanges
New Files
src/tag/commands/security-review.tssrc/tag/commands/security-scan.tssrc/create-prompt/templates/security-review-prompt.tssrc/create-prompt/templates/security-report-prompt.tsThis PR is part of a split from the security review feature branch.
PR Stack