Skip to content

test: combined vulnerability demo for semgrep + LLM review#23

Open
ramirobarraco wants to merge 1 commit intomainfrom
test/combined-vulnerabilities
Open

test: combined vulnerability demo for semgrep + LLM review#23
ramirobarraco wants to merge 1 commit intomainfrom
test/combined-vulnerabilities

Conversation

@ramirobarraco
Copy link
Copy Markdown
Collaborator

Summary

Combined demo PR showcasing both semgrep detection and LLM review capabilities.

Vulnerabilities Included

src/vulnerable_demo.py (6 issues):

  • SQL injection (f-string in query)
  • Command injection (subprocess shell=True)
  • Hardcoded secrets (API key, password)
  • Insecure deserialization (pickle.loads)
  • Path traversal (unvalidated file path)
  • Arbitrary code execution (exec)

src/config.py (1 issue):

  • Path traversal via user-controlled cache_dir/cache_name

Expected Results

  1. Semgrep should detect ~6 issues (SQL, command injection, pickle, exec, etc.)
  2. LLM should find additional issues semgrep misses (hardcoded secrets, path traversal logic)
  3. Separate comments: Static analysis in one comment, LLM review in another
  4. Cost: ~$0.005 with Gemini 2.5 Flash

⚠️ WARNING: Contains intentionally vulnerable code. DO NOT MERGE.

🤖 Generated with Claude Code

Combines vulnerabilities from both test PRs:

1. src/vulnerable_demo.py - Security vulnerability examples:
   - SQL injection
   - Command injection (shell=True)
   - Hardcoded secrets
   - Insecure deserialization (pickle)
   - Path traversal
   - Arbitrary code execution (exec)

2. src/config.py - Path traversal vulnerability:
   - User-controlled cache_dir and cache_name joined without validation
   - Attacker could write files outside intended directory

WARNING: This is for demonstration purposes only. DO NOT MERGE.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

🔍 Nitpick Senior - Static Analysis

Found 5 issue(s) via static analysis (semgrep):

Severity File Line Rule Message
❌ ERROR src/vulnerable_demo.py 19 python.sqlalchemy.security.sqlalchemy-execute-raw-query.sqlalchemy-execute-raw-query Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL...
❌ ERROR src/vulnerable_demo.py 28 python.lang.security.audit.subprocess-shell-true.subprocess-shell-true Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn ...
⚠️ WARNING src/vulnerable_demo.py 19 python.lang.security.audit.formatted-sql-query.formatted-sql-query Detected possible formatted SQL query. Use parameterized queries instead.
⚠️ WARNING src/vulnerable_demo.py 46 python.lang.security.deserialization.pickle.avoid-pickle Avoid using pickle, which is known to lead to code execution vulnerabilities. When unpickling, the...
⚠️ WARNING src/vulnerable_demo.py 61 python.lang.security.audit.exec-detected.exec-detected Detected the use of exec(). exec() can be dangerous if used to evaluate dynamic content. If this con...

💡 These findings are from automated static analysis. The AI review below focuses on issues requiring human judgment.


🤓 Um, actually... reviewed by Nitpick Senior

Comment thread src/vulnerable_demo.py


def hardcoded_secret_example():
"""Contains hardcoded credentials - security risk."""
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security | ❌ ERROR

Sensitive credentials, such as an API key and password, are hardcoded directly in the source code.

Why this matters: Embedding sensitive credentials directly into the source code makes them easily discoverable by anyone with access to the codebase, including version control history. This practice violates the principle of least privilege and significantly increases the risk of credential compromise, as these secrets are not managed securely and can be exposed in logs, build artifacts, or public repositories.

@github-actions
Copy link
Copy Markdown

🤓 Nitpick Senior Review

This pull request introduces a new configuration option for a cache path and adds a demo file showcasing various vulnerabilities. The cache path configuration is susceptible to path traversal, and the demo file contains hardcoded secrets that were not flagged by the provided static analysis findings.

Confidence: 2/5

⚠️ Changes needed - significant issues

Files Changed

File Type Overview
src/config.py Bug fix Adds a new cache_path configuration, which is vulnerable to path traversal due to unvalidated environment variable input.
src/vulnerable_demo.py Enhancement A new file demonstrating various vulnerabilities, including hardcoded secrets, intended for static analysis showcasing.

Issues Found (1)


🤓 Um, actually... reviewed by Nitpick Senior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant