-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add @droid security commands #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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>
dff43cf to
01e9df8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
varin-nair-factory
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approving to unblock, but pls see comments
|
Droid encountered an error —— View job |
Summary
Add support for security-focused code review commands.
Closes https://linear.app/factoryai/issue/FAC-14344/add-security-command-parser-and-trigger-detection
Closes https://linear.app/factoryai/issue/FAC-14355/create-security-review-prompt-template
Closes https://linear.app/factoryai/issue/FAC-14356/implement-preparesecuritymode-command-handler
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