Skip to content

feat: add Python/async/SSRF patterns to review checklist#531

Open
keshav55 wants to merge 1 commit intogarrytan:mainfrom
keshav55:feat/python-review-patterns
Open

feat: add Python/async/SSRF patterns to review checklist#531
keshav55 wants to merge 1 commit intogarrytan:mainfrom
keshav55:feat/python-review-patterns

Conversation

@keshav55
Copy link
Copy Markdown

Summary

Adds Python-specific patterns to review/checklist.md. Every pattern here caught a real bug on a production Python/FastAPI/Supabase codebase.

CRITICAL additions

  • Shell Injection (Python): subprocess.run(shell=True) with f-strings, os.system(), eval()/exec() on LLM output
  • LLM Trust Boundary: SSRF via LLM-generated URLs, stored prompt injection via knowledge base writes

INFORMATIONAL additions

  • Async/Sync Mixing: blocking calls in async endpoints (subprocess.run, open(), requests.get inside async def)
  • Column/Field Name Safety: wrong column names in ORM queries silently return empty results

Evidence

These patterns were discovered by applying the existing checklist to a production codebase:

  • Shell injection in subprocess.run(f"grep -rl '{name}'...", shell=True)real bug, fixed
  • Wrong column name .gte("created_at", ...) on a table using timestamp3 endpoints silently returning 0, fixed
  • SSRF: LLM-provided URL fetched without validation, content stored as agent knowledge — real vulnerability, fixed

Test plan

  • 365/365 skill validation tests pass
  • No conflicts with existing checklist categories

🤖 Generated with Claude Code

Adds three new categories to the pre-landing review checklist:

CRITICAL:
- Shell Injection (Python): subprocess.run(shell=True) with f-strings,
  os.system() with interpolation, eval()/exec() on LLM output
- LLM Trust Boundary: SSRF via LLM-generated URLs, stored prompt
  injection via knowledge base writes

INFORMATIONAL:
- Async/Sync Mixing: blocking calls in async endpoints, time.sleep()
  in async, sync DB calls without run_in_executor()
- Column/Field Name Safety: wrong column names in ORM queries silently
  return empty results (real bug found on a production codebase)

These patterns were discovered by applying the existing checklist to a
Python/FastAPI/Supabase codebase — every pattern here caught a real bug.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@keshav55 keshav55 force-pushed the feat/python-review-patterns branch from e893e77 to 38c95b6 Compare March 26, 2026 23:50
garrytan added a commit that referenced this pull request Mar 27, 2026
garrytan added a commit that referenced this pull request Mar 27, 2026
…odex fallback, Windows fix, Python patterns (v0.12.9.0) (#561)

* fix: sync package.json version with VERSION file (0.12.7.0)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* perf: shallow clone for faster install (#484)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* feat: Python/async/SSRF patterns in review checklist (#531)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* feat: namespace skill symlinks with gstack- prefix (#503)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* feat: add uninstall script (#323)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* feat: office-hours Claude subagent fallback when Codex unavailable (#464)

Updates generateCodexSecondOpinion resolver to always offer second opinion
and fall back to Claude subagent when Codex is unavailable or errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix: findPort() race condition via net.createServer (#490)

Replaces Bun.serve() port probing with net.createServer() for proper
async bind/close semantics. Fixes Windows EADDRINUSE race condition.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* test: add tests for uninstall, setup prefix, and resolver fallback

- Uninstall integration tests: syntax, flags, mock install layout, upgrade path
- Setup prefix tests: gstack-* prefixing, --no-prefix, cleanup migration
- Resolver tests: Claude subagent fallback in generated SKILL.md

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* chore: bump version and changelog (v0.12.9.0)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
@keshav55
Copy link
Copy Markdown
Author

Hey Garry — stoked the Python patterns made it into v0.12.9.0! We've been deep in gstack for the past few days and love what you've built.

Quick thought on the wave merge process: when community PRs get squashed into a single commit, the original contributors lose their green squares on GitHub. Might be worth adding Co-Authored-By trailers to wave commits — it's a small thing but contributor graphs are a big motivator for open source contributors. People see "I contributed to a 50K-star repo" on their profile and it drives more PRs.

For the v0.12.9.0 wave, something like:
Co-Authored-By: keshav55 <[email protected]>

CONTRIBUTING.md already mentions preserving attributions in wave PRs, so this would just be closing the loop on that.

We have 3 more PRs open (#529, #530, #532) and would love to keep contributing. Incredible project.

rapidstartup pushed a commit to rapidstartup/gstack that referenced this pull request Mar 29, 2026
…odex fallback, Windows fix, Python patterns (v0.12.9.0) (garrytan#561)

* fix: sync package.json version with VERSION file (0.12.7.0)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* perf: shallow clone for faster install (garrytan#484)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* feat: Python/async/SSRF patterns in review checklist (garrytan#531)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* feat: namespace skill symlinks with gstack- prefix (garrytan#503)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* feat: add uninstall script (garrytan#323)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* feat: office-hours Claude subagent fallback when Codex unavailable (garrytan#464)

Updates generateCodexSecondOpinion resolver to always offer second opinion
and fall back to Claude subagent when Codex is unavailable or errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix: findPort() race condition via net.createServer (garrytan#490)

Replaces Bun.serve() port probing with net.createServer() for proper
async bind/close semantics. Fixes Windows EADDRINUSE race condition.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* test: add tests for uninstall, setup prefix, and resolver fallback

- Uninstall integration tests: syntax, flags, mock install layout, upgrade path
- Setup prefix tests: gstack-* prefixing, --no-prefix, cleanup migration
- Resolver tests: Claude subagent fallback in generated SKILL.md

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* chore: bump version and changelog (v0.12.9.0)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
24601 pushed a commit to 24601/gastack that referenced this pull request Mar 29, 2026
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