chore(agents): Add security-review skill to agent configuration#5498
Open
ericapisani wants to merge 1 commit intomasterfrom
Open
chore(agents): Add security-review skill to agent configuration#5498ericapisani wants to merge 1 commit intomasterfrom
ericapisani wants to merge 1 commit intomasterfrom
Conversation
Enable the security-review skill in Warden to provide AI-powered security analysis on pull requests. The skill reviews code for vulnerabilities and security issues, triggered on PR open and updates. Configures the skill in agents.toml, agents.lock, and warden.toml with triggers for opened, synchronize, and reopened actions. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes 🐛
Documentation 📚
Internal Changes 🔧Agents
Other
🤖 This preview updates automatically when you update the PR. |
Contributor
Codecov Results 📊✅ 13 passed | Total: 13 | Pass Rate: 100% | Execution Time: 7.75s All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 13701 uncovered lines. Files with missing lines (180)
Generated by Codecov Action |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| target = (base / user_path).resolve() | ||
|
|
||
| # Verify target is under base | ||
| if not str(target).startswith(str(base)): |
Contributor
There was a problem hiding this comment.
Path traversal prevention example can be bypassed with sibling directory names
The safe_join function example uses str(target).startswith(str(base)) to verify the resolved path is under the base directory. This check is flawed: if base is /uploads and an attacker crafts a path resolving to /uploads_malicious/file, the startswith check passes because /uploads_malicious.startswith(/uploads) is True. Developers copying this security guidance may inadvertently introduce path traversal vulnerabilities.
Suggested fix: Append os.sep to the base path before comparison, or compare using os.path.commonpath to ensure true containment.
Suggested change
| if not str(target).startswith(str(base)): | |
| if not str(target).startswith(str(base) + os.sep): |
Also found at 2 additional locations
.agents/skills/security-review/references/modern-threats.md:373-373.agents/skills/security-review/references/modern-threats.md:334-334
Identified by Warden [find-bugs] · RVP-GBQ
| --- | ||
| name: security-review | ||
| description: Security code review for vulnerabilities. Use when asked to "security review", "find vulnerabilities", "check for security issues", "audit security", "OWASP review", or review code for injection, XSS, authentication, authorization, cryptography issues. Provides systematic review with confidence-based reporting. | ||
| allowed-tools: Read, Grep, Glob, Bash, Task |
Contributor
There was a problem hiding this comment.
Bash tool granted without clear justification in skill instructions
The skill grants Bash tool access but has no scripts directory and the SKILL.md instructions don't reference any CLI tools, linters, shell commands, or scripts that would require Bash execution. This violates the least privilege principle. Per permission-analysis.md, Bash is justified when 'Running bundled scripts, git/gh CLI, build tools' but unjustified when there are 'No scripts or CLI commands in instructions'.
Suggested fix: Remove Bash from allowed-tools unless there's a specific need for shell access
Suggested change
| allowed-tools: Read, Grep, Glob, Bash, Task | |
| allowed-tools: Read, Grep, Glob, Task |
Also found at 1 additional location
warden.toml:54
Identified by Warden [skill-scanner] · J23-EH8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enable the security-review skill in Warden to provide AI-powered security analysis on pull requests. The skill reviews code for vulnerabilities and security issues, triggered on PR open and updates.
Configures the skill in agents.toml, agents.lock, and warden.toml with triggers for opened, synchronize, and reopened actions.