refactor: split into 3 repos + Trail of Bits + pyclaude-forge integration#35
Open
refactor: split into 3 repos + Trail of Bits + pyclaude-forge integration#35
Conversation
- Removed .claude/ (skills, agents, rules, hooks, settings) -> stranma/claude-code-harness - Removed .devcontainer/ (Dockerfile, firewall, devcontainer.json) -> stranma/claude-code-devcontainer - Removed security hooks and tests (dangerous-actions-blocker, permissions, firewall tamper) - Removed AGENTS.md (duplicate), docs/DECISIONS.md, docs/DEVELOPMENT_PROCESS.md - Slimmed CLAUDE.md to template-only dev commands - Added .ruff_cache/, .agents/, .tmp_*/ to .gitignore - 15 template tests still pass Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Added .ruff_cache/, .agents/, .tmp_*/ to .gitignore - setup_project.py: mkdir .devcontainer/ before writing docker-compose.yml (directory no longer ships with template after devcontainer extraction) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove stale files (PDFs, IMPLEMENTATION_PLAN stub, .dockerignore) - Add --devcontainer trailofbits and --egress-firewall to setup_project.py - Rewrite README with composition diagram showing 3 external components - Update CLAUDE.md with correct repo URLs - Update GETTING_STARTED.md for split architecture - Update CHANGELOG with split summary Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
shutil.rmtree fails on Windows when removing .git/objects/pack/*.idx files because they're marked read-only. Add onerror handler to chmod before unlink. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughRemoves the internal Claude Code workflow (agents, skills, rules, hooks, settings, devcontainer, and related tests/docs) and shifts the repository to a composable template architecture; also adds devcontainer composition support to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/GETTING_STARTED.md`:
- Around line 115-125: The descriptions for "agents" and "hooks" read as
built-in features but actually require installing pyclaude-forge; update the
"What are agents?" and "What are hooks?" paragraphs to add a short clarifying
sentence that these features depend on pyclaude-forge being installed (and point
readers to the "Next Steps" install section or the pyclaude-forge installation
instruction), so users know to install pyclaude-forge before expecting
agents/hooks to be available; leave the "devcontainer" text as-is.
In `@setup_project.py`:
- Around line 477-482: The FIREWALL_GIST_URL constant currently points at the
gist raw endpoint without a revision, which can silently change; update the
FIREWALL_GIST_URL to include a specific gist commit SHA (pin the URL to
.../raw/<COMMIT_SHA>/init-firewall.sh) or, if intentionally using latest, add a
clear comment and README note near the FIREWALL_GIST_URL declaration explaining
that the script is intentionally unpinned and may change; locate and modify the
FIREWALL_GIST_URL variable in this file (and any references to init-firewall.sh)
to apply the pinned URL or add the documentation comment.
- Around line 526-540: The downloaded firewall script is saved to firewall_path
(.devcontainer/init-firewall.sh) but the code sets dc_json["postStartCommand"] =
"sudo /usr/local/bin/init-firewall.sh", causing a path mismatch; fix by ensuring
the postStartCommand points to the actual location or by moving the script into
/usr/local/bin during setup: update the postStartCommand value written by the
block that modifies dc_json to reference the .devcontainer/init-firewall.sh path
(or add logic after urllib.request.urlretrieve to copy firewall_path to
/usr/local/bin and set executable bits), referencing firewall_path, dc_json,
postStartCommand, and init-firewall.sh in your changes.
- Around line 543-544: The broad except Exception in the firewall script fetch
block should be narrowed to only the expected I/O/network errors: catch the
specific exceptions raised by the fetch code (e.g.,
requests.exceptions.RequestException and/or urllib.error.URLError and OSError)
and append the warning to actions; for anything else re-raise the exception so
programming errors aren’t swallowed. Locate the try/except around the firewall
script download (the block that appends to actions on failure) and replace the
generic except with a tuple of those specific exception types, or add an
explicit raise for unexpected exceptions after handling the expected ones.
- Around line 752-759: The current flow in setup_project.py sets dc_type =
config.get("devcontainer", "none") and then calls setup_devcontainer(...) which
overwrites the .devcontainer directory, clobbering any prior Docker Compose
service profile created when services were enabled; add a guard in the logic
that detects the conflicting options (the devcontainer option value
"trailofbits" and whatever config key or flag enables services) and either (a)
raise a clear error/exit or print a warning and skip one of the actions, or (b)
implement a merge path inside setup_devcontainer to read an existing
.devcontainer/devcontainer.json and merge the Docker Compose "service" profile
into the Trail of Bits template instead of overwriting—use the dc_type variable
and the setup_devcontainer(...) call as the insertion point to perform the
check/merge and ensure actions printed reflect the chosen resolution.
- Around line 512-515: shutil.rmtree is using the deprecated onerror parameter;
update the deletion call to be compatible with Python 3.12+ by creating a
compatibility wrapper that uses onexc when available and falls back to onerror
otherwise: keep the existing _remove_readonly handler and pass it via a small
helper that checks shutil.rmtree signature or sys.version_info (e.g., if Python
>= 3.12 use onexc=_remove_readonly else onerror=_remove_readonly) when calling
shutil.rmtree(git_dir, ...), ensuring _remove_readonly, shutil.rmtree and
git_dir are referenced unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 59b10ea9-c455-4f96-84fe-020a399a4ed4
⛔ Files ignored due to path filters (2)
docs/Sabotage Risk Report Claude Opus 4.6.pdfis excluded by!**/*.pdfdocs/evaluating_agents_md.pdfis excluded by!**/*.pdf
📒 Files selected for processing (38)
.claude/agents/code-quality-validator.md.claude/agents/code-reviewer.md.claude/agents/docs-updater.md.claude/agents/pr-writer.md.claude/agents/review-responder.md.claude/agents/test-coverage-validator.md.claude/deploy.json.example.claude/hooks/auto-format.sh.claude/hooks/dangerous-actions-blocker.sh.claude/rules/architecture-review.md.claude/rules/code-quality-review.md.claude/rules/performance-review.md.claude/rules/test-review.md.claude/settings.json.claude/settings.local.json.example.claude/skills/design/SKILL.md.claude/skills/done/SKILL.md.claude/skills/landed/SKILL.md.claude/skills/sync/SKILL.md.devcontainer/Dockerfile.devcontainer/devcontainer.json.devcontainer/init-firewall.sh.dockerignore.github/workflows/template-sync.yml.gitignoreCLAUDE.mdREADME.mddocs/CHANGELOG.mddocs/DECISIONS.mddocs/DEVELOPMENT_PROCESS.mddocs/GETTING_STARTED.mddocs/IMPLEMENTATION_PLAN.mdsetup_project.pytests/test_agents.pytests/test_hooks.pytests/test_permissions.pytests/test_rules.pytests/test_skills.py
💤 Files with no reviewable changes (31)
- .claude/deploy.json.example
- .claude/rules/code-quality-review.md
- .claude/rules/architecture-review.md
- .claude/agents/test-coverage-validator.md
- docs/IMPLEMENTATION_PLAN.md
- .claude/agents/pr-writer.md
- .claude/hooks/auto-format.sh
- .dockerignore
- .claude/rules/test-review.md
- .claude/settings.local.json.example
- .claude/rules/performance-review.md
- docs/DECISIONS.md
- .claude/agents/review-responder.md
- .claude/agents/docs-updater.md
- .claude/skills/design/SKILL.md
- .claude/skills/sync/SKILL.md
- .devcontainer/Dockerfile
- .devcontainer/devcontainer.json
- docs/DEVELOPMENT_PROCESS.md
- .claude/skills/landed/SKILL.md
- .claude/settings.json
- tests/test_rules.py
- tests/test_agents.py
- tests/test_skills.py
- .claude/hooks/dangerous-actions-blocker.sh
- tests/test_hooks.py
- .claude/agents/code-reviewer.md
- .devcontainer/init-firewall.sh
- .claude/skills/done/SKILL.md
- .claude/agents/code-quality-validator.md
- tests/test_permissions.py
Comment on lines
115
to
+125
| ### What are agents? | ||
|
|
||
| Agents are specialized Claude Code sub-processes that handle specific tasks (linting, testing, code review). They run automatically as part of the workflow. You don't invoke them directly -- `/done` orchestrates them. | ||
| Specialized Claude Code sub-processes for specific tasks (linting, testing, code review). They run automatically as part of `/done`. | ||
|
|
||
| ### What are hooks? | ||
|
|
||
| Shell scripts that run before or after Claude Code actions. For example, the `dangerous-actions-blocker` hook prevents Claude from running `rm -rf` or leaking secrets. They run silently in the background. | ||
|
|
||
| ### What is TDD? | ||
|
|
||
| Test-Driven Development: write the test first, then write the code to make it pass. The template enforces this order. It feels backwards at first, but it produces more reliable code because you're always building toward a defined expectation. | ||
| Shell scripts that run before or after Claude Code actions. For example, `auto-format` runs ruff after edits. | ||
|
|
||
| ### What is a devcontainer? | ||
|
|
||
| A Docker container configured for development. VS Code opens your project inside the container, so all tools are pre-installed and Claude Code runs in a sandbox. If something goes wrong, your host machine is unaffected. | ||
| A Docker container configured for development. VS Code opens your project inside it, so all tools are pre-installed and Claude Code runs in a sandbox. |
There was a problem hiding this comment.
Concept descriptions imply built-in features that require pyclaude-forge.
Lines 117 and 121 describe agents and hooks as if they're part of the template, but they actually require installing pyclaude-forge (mentioned later in Next Steps). Consider adding a note that these features require pyclaude-forge installation.
Suggested clarification
### What are agents?
-Specialized Claude Code sub-processes for specific tasks (linting, testing, code review). They run automatically as part of `/done`.
+Specialized Claude Code sub-processes for specific tasks (linting, testing, code review). They run automatically as part of `/done`. Requires [pyclaude-forge](https://github.com/stranma/pyclaude-forge).
### What are hooks?
-Shell scripts that run before or after Claude Code actions. For example, `auto-format` runs ruff after edits.
+Shell scripts that run before or after Claude Code actions. For example, `auto-format` runs ruff after edits. Requires [pyclaude-forge](https://github.com/stranma/pyclaude-forge).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ### What are agents? | |
| Agents are specialized Claude Code sub-processes that handle specific tasks (linting, testing, code review). They run automatically as part of the workflow. You don't invoke them directly -- `/done` orchestrates them. | |
| Specialized Claude Code sub-processes for specific tasks (linting, testing, code review). They run automatically as part of `/done`. | |
| ### What are hooks? | |
| Shell scripts that run before or after Claude Code actions. For example, the `dangerous-actions-blocker` hook prevents Claude from running `rm -rf` or leaking secrets. They run silently in the background. | |
| ### What is TDD? | |
| Test-Driven Development: write the test first, then write the code to make it pass. The template enforces this order. It feels backwards at first, but it produces more reliable code because you're always building toward a defined expectation. | |
| Shell scripts that run before or after Claude Code actions. For example, `auto-format` runs ruff after edits. | |
| ### What is a devcontainer? | |
| A Docker container configured for development. VS Code opens your project inside the container, so all tools are pre-installed and Claude Code runs in a sandbox. If something goes wrong, your host machine is unaffected. | |
| A Docker container configured for development. VS Code opens your project inside it, so all tools are pre-installed and Claude Code runs in a sandbox. | |
| ### What are agents? | |
| Specialized Claude Code sub-processes for specific tasks (linting, testing, code review). They run automatically as part of `/done`. Requires [pyclaude-forge](https://github.com/stranma/pyclaude-forge). | |
| ### What are hooks? | |
| Shell scripts that run before or after Claude Code actions. For example, `auto-format` runs ruff after edits. Requires [pyclaude-forge](https://github.com/stranma/pyclaude-forge). | |
| ### What is a devcontainer? | |
| A Docker container configured for development. VS Code opens your project inside it, so all tools are pre-installed and Claude Code runs in a sandbox. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/GETTING_STARTED.md` around lines 115 - 125, The descriptions for
"agents" and "hooks" read as built-in features but actually require installing
pyclaude-forge; update the "What are agents?" and "What are hooks?" paragraphs
to add a short clarifying sentence that these features depend on pyclaude-forge
being installed (and point readers to the "Next Steps" install section or the
pyclaude-forge installation instruction), so users know to install
pyclaude-forge before expecting agents/hooks to be available; leave the
"devcontainer" text as-is.
setup_project.py
Outdated
Comment on lines
+477
to
+482
| FIREWALL_GIST_URL = ( | ||
| "https://gist.githubusercontent.com/stranma/" | ||
| "f43d932bedc8335e24404c9784fcf190/raw/init-firewall.sh" | ||
| ) | ||
|
|
||
| TOB_REPO = "https://github.com/trailofbits/claude-code-devcontainer.git" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Gist URL lacks version pinning.
The firewall script is fetched from a gist's /raw/ endpoint, which always returns the latest revision. If the gist is updated or deleted, users get unexpected behavior silently.
Consider pinning to a specific gist revision:
https://gist.githubusercontent.com/stranma/f43d932bedc8335e24404c9784fcf190/raw/<COMMIT_SHA>/init-firewall.sh
Alternatively, document that the firewall script version is intentionally "latest."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@setup_project.py` around lines 477 - 482, The FIREWALL_GIST_URL constant
currently points at the gist raw endpoint without a revision, which can silently
change; update the FIREWALL_GIST_URL to include a specific gist commit SHA (pin
the URL to .../raw/<COMMIT_SHA>/init-firewall.sh) or, if intentionally using
latest, add a clear comment and README note near the FIREWALL_GIST_URL
declaration explaining that the script is intentionally unpinned and may change;
locate and modify the FIREWALL_GIST_URL variable in this file (and any
references to init-firewall.sh) to apply the pinned URL or add the documentation
comment.
Comment on lines
+512
to
+515
| def _remove_readonly(func, path, _): | ||
| os.chmod(path, 0o700) | ||
| func(path) | ||
| shutil.rmtree(git_dir, onerror=_remove_readonly) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Python 3.12 shutil.rmtree onerror deprecation
💡 Result:
Web search failed: Server error: no LLM provider could handle the message
🏁 Script executed:
fd -type f -name "setup_project.py"Repository: stranma/claude-code-python-template
Length of output: 249
🏁 Script executed:
fd -t f -name "setup_project.py"Repository: stranma/claude-code-python-template
Length of output: 309
🏁 Script executed:
fd "setup_project.py"Repository: stranma/claude-code-python-template
Length of output: 123
🏁 Script executed:
wc -l setup_project.pyRepository: stranma/claude-code-python-template
Length of output: 99
🏁 Script executed:
sed -n '510,520p' setup_project.pyRepository: stranma/claude-code-python-template
Length of output: 662
🏁 Script executed:
cat -n setup_project.py | sed -n '512,515p'Repository: stranma/claude-code-python-template
Length of output: 297
🏁 Script executed:
fd "setup.py|pyproject.toml|setup.cfg|tox.ini"Repository: stranma/claude-code-python-template
Length of output: 145
🏁 Script executed:
cat pyproject.tomlRepository: stranma/claude-code-python-template
Length of output: 1318
🏁 Script executed:
cat tests/test_setup_project.py | head -100Repository: stranma/claude-code-python-template
Length of output: 3426
🏁 Script executed:
python3 << 'EOF'
import shutil
import inspect
# Get the signature of rmtree
sig = inspect.signature(shutil.rmtree)
print("shutil.rmtree signature:")
print(sig)
print("\nParameters:")
for param_name, param in sig.parameters.items():
print(f" {param_name}: {param}")
EOFRepository: stranma/claude-code-python-template
Length of output: 270
🏁 Script executed:
python3 << 'EOF'
import sys
print(f"Python version: {sys.version}")
print(f"Version info: {sys.version_info}")
# Check Python 3.12 release notes or documentation
# Try importing shutil and checking for onexc parameter
import shutil
import inspect
sig = inspect.signature(shutil.rmtree)
params = list(sig.parameters.keys())
print(f"\nshutil.rmtree parameters: {params}")
print(f"Has 'onerror': {'onerror' in params}")
print(f"Has 'onexc': {'onexc' in params}")
EOFRepository: stranma/claude-code-python-template
Length of output: 348
🏁 Script executed:
python3 << 'EOF'
# Check Python documentation for 3.12 changes
# Based on training knowledge, Python 3.12 changed shutil.rmtree API
# Let me verify if there are any relevant comments in the codebase about Python 3.12
import subprocess
result = subprocess.run(['grep', '-r', '3.12', '--include=*.py', '--include=*.md'],
capture_output=True, text=True)
print("Mentions of 3.12 in codebase:")
print(result.stdout if result.stdout else "(none found)")
EOFRepository: stranma/claude-code-python-template
Length of output: 121
🏁 Script executed:
python3 << 'EOF'
# Check Python 3.12 documentation reference
# Based on PEP 594 and Python 3.12 release notes, shutil.rmtree()
# signature changed in Python 3.12
# Let me check the actual implementation details
import sys
import shutil
print("Current Python version:", sys.version_info)
print("\nFrom Python 3.12 release notes:")
print("shutil.rmtree() gained a new 'onexc' parameter to replace 'onerror'")
print("The 'onerror' parameter is deprecated as of Python 3.12")
print("\nChange summary:")
print("- Python 3.11 and earlier: uses onerror(func, path, exc_info)")
print("- Python 3.12+: uses onexc(func, path, exc)")
print("\nNote: onexc receives exception object, not exc_info tuple")
EOFRepository: stranma/claude-code-python-template
Length of output: 518
shutil.rmtree onerror parameter deprecated in Python 3.12.
The onerror parameter is deprecated in Python 3.12+; use onexc instead. Since the template targets Python >=3.11,<3.13, add a compatibility wrapper.
Proposed fix for Python 3.12+ compatibility
+import sys
+
def _remove_readonly(func, path, _):
os.chmod(path, 0o700)
func(path)
-shutil.rmtree(git_dir, onerror=_remove_readonly)
+if sys.version_info >= (3, 12):
+ shutil.rmtree(git_dir, onexc=lambda f, p, e: _remove_readonly(f, p, None))
+else:
+ shutil.rmtree(git_dir, onerror=_remove_readonly)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@setup_project.py` around lines 512 - 515, shutil.rmtree is using the
deprecated onerror parameter; update the deletion call to be compatible with
Python 3.12+ by creating a compatibility wrapper that uses onexc when available
and falls back to onerror otherwise: keep the existing _remove_readonly handler
and pass it via a small helper that checks shutil.rmtree signature or
sys.version_info (e.g., if Python >= 3.12 use onexc=_remove_readonly else
onerror=_remove_readonly) when calling shutil.rmtree(git_dir, ...), ensuring
_remove_readonly, shutil.rmtree and git_dir are referenced unchanged.
Comment on lines
+526
to
+540
| firewall_path = dc_dir / "init-firewall.sh" | ||
| try: | ||
| urllib.request.urlretrieve(FIREWALL_GIST_URL, firewall_path) | ||
| firewall_path.chmod(firewall_path.stat().st_mode | 0o755) | ||
| actions.append(" Fetched egress firewall (init-firewall.sh) from gist") | ||
|
|
||
| # Add postStartCommand to devcontainer.json if it exists | ||
| dc_json = dc_dir / "devcontainer.json" | ||
| if dc_json.exists(): | ||
| raw = dc_json.read_text(encoding="utf-8") | ||
| try: | ||
| config = json.loads(raw) | ||
| config["postStartCommand"] = "sudo /usr/local/bin/init-firewall.sh" | ||
| dc_json.write_text(json.dumps(config, indent=2) + "\n", encoding="utf-8") | ||
| actions.append(" Added postStartCommand for firewall to devcontainer.json") |
There was a problem hiding this comment.
Path mismatch: firewall script written to wrong location.
The script is downloaded to .devcontainer/init-firewall.sh (line 526), but postStartCommand references /usr/local/bin/init-firewall.sh (line 538). The container will fail to find the script at runtime.
Proposed fix
Either update the postStartCommand to reference the actual location:
- config["postStartCommand"] = "sudo /usr/local/bin/init-firewall.sh"
+ config["postStartCommand"] = "sudo .devcontainer/init-firewall.sh"Or copy the script to /usr/local/bin/ during container setup (via a Dockerfile COPY or postCreateCommand).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@setup_project.py` around lines 526 - 540, The downloaded firewall script is
saved to firewall_path (.devcontainer/init-firewall.sh) but the code sets
dc_json["postStartCommand"] = "sudo /usr/local/bin/init-firewall.sh", causing a
path mismatch; fix by ensuring the postStartCommand points to the actual
location or by moving the script into /usr/local/bin during setup: update the
postStartCommand value written by the block that modifies dc_json to reference
the .devcontainer/init-firewall.sh path (or add logic after
urllib.request.urlretrieve to copy firewall_path to /usr/local/bin and set
executable bits), referencing firewall_path, dc_json, postStartCommand, and
init-firewall.sh in your changes.
Comment on lines
+543
to
+544
| except Exception as e: | ||
| actions.append(f" WARNING: Failed to fetch firewall script: {e}") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Broad except Exception catches too much.
This catches KeyboardInterrupt (via BaseException inheritance path — actually Exception doesn't catch KeyboardInterrupt, so this is fine). However, it still catches unexpected errors like MemoryError or programming errors, masking real issues.
Consider catching more specific exceptions:
Proposed refinement
- except Exception as e:
+ except (urllib.error.URLError, OSError) as e:
actions.append(f" WARNING: Failed to fetch firewall script: {e}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@setup_project.py` around lines 543 - 544, The broad except Exception in the
firewall script fetch block should be narrowed to only the expected I/O/network
errors: catch the specific exceptions raised by the fetch code (e.g.,
requests.exceptions.RequestException and/or urllib.error.URLError and OSError)
and append the warning to actions; for anything else re-raise the exception so
programming errors aren’t swallowed. Locate the try/except around the firewall
script download (the block that appends to actions on failure) and replace the
generic except with a tuple of those specific exception types, or add an
explicit raise for unexpected exceptions after handling the expected ones.
Comment on lines
+752
to
+759
| # Step 5b: Set up devcontainer | ||
| dc_type = config.get("devcontainer", "none") | ||
| if dc_type != "none": | ||
| print(f"\nSetting up devcontainer ({dc_type})...") | ||
| fw = config.get("egress_firewall", False) | ||
| actions = setup_devcontainer(TEMPLATE_DIR, devcontainer=dc_type, egress_firewall=fw) | ||
| for a in actions: | ||
| print(a) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Test what happens when both --services and --devcontainer are used
# This is a read-only check of the code logic
# Check if there's any merge logic between services and devcontainer setup
rg -n "services.*devcontainer|devcontainer.*services" setup_project.py
# Check the order of operations in main()
rg -n "Step 5|services|devcontainer" setup_project.py | head -30Repository: stranma/claude-code-python-template
Length of output: 2827
🏁 Script executed:
#!/bin/bash
# Get context around lines 752-759 and the broader Step 5/5b logic
sed -n '740,790p' setup_project.py | cat -nRepository: stranma/claude-code-python-template
Length of output: 2538
🏁 Script executed:
#!/bin/bash
# Check the main() function to see guard conditions and ordering
rg -n "def main" setup_project.py -A 200 | head -100Repository: stranma/claude-code-python-template
Length of output: 4910
🏁 Script executed:
#!/bin/bash
# See the full setup_devcontainer() implementation to understand what it does with devcontainer.json
sed -n '485,550p' setup_project.py | cat -nRepository: stranma/claude-code-python-template
Length of output: 3452
🏁 Script executed:
#!/bin/bash
# Check if there are any guards that prevent both --services and --devcontainer from being used together
rg -n "services|devcontainer" setup_project.py | grep -E "(if|else|and|or|==|!)" | head -20Repository: stranma/claude-code-python-template
Length of output: 1330
Confirmed conflict: --services and --devcontainer trailofbits will overwrite each other.
When both options are enabled, Step 5 creates or modifies .devcontainer/devcontainer.json for the Docker Compose service profile, but Step 5b immediately overwrites the entire .devcontainer/ directory by cloning the Trail of Bits repository. The resulting configuration will be inconsistent — the services configuration from Step 5 is lost.
The code does not prevent or warn about using both options together. Consider:
- Adding a guard condition to prevent both options from being enabled simultaneously, or
- Merging the devcontainer.json configurations instead of overwriting them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@setup_project.py` around lines 752 - 759, The current flow in
setup_project.py sets dc_type = config.get("devcontainer", "none") and then
calls setup_devcontainer(...) which overwrites the .devcontainer directory,
clobbering any prior Docker Compose service profile created when services were
enabled; add a guard in the logic that detects the conflicting options (the
devcontainer option value "trailofbits" and whatever config key or flag enables
services) and either (a) raise a clear error/exit or print a warning and skip
one of the actions, or (b) implement a merge path inside setup_devcontainer to
read an existing .devcontainer/devcontainer.json and merge the Docker Compose
"service" profile into the Trail of Bits template instead of overwriting—use the
dc_type variable and the setup_devcontainer(...) call as the insertion point to
perform the check/merge and ensure actions printed reflect the chosen
resolution.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
setup_project.py (4)
744-756:⚠️ Potential issue | 🟠 MajorBlock the unsupported
--services+--devcontainer trailofbitscombination.Step 5 creates
.devcontainer/, then Step 5b tries togit cloneinto the same path. Once service files exist, the clone destination is no longer empty, so the requested Trail of Bits devcontainer is skipped. Guard the combination up front or implement a real merge path.Possible guard
services = config.get("services", "none") + dc_type = config.get("devcontainer", "none") + if services != "none" and dc_type == "trailofbits": + print("Error: --services cannot be combined with --devcontainer trailofbits yet.") + sys.exit(1) if services != "none": print(f"\nConfiguring devcontainer services ({services})...") actions = configure_devcontainer_services(TEMPLATE_DIR, services, replacements) for a in actions: print(a) # Step 5b: Set up devcontainer - dc_type = config.get("devcontainer", "none") if dc_type != "none":🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup_project.py` around lines 744 - 756, Prevent the unsupported combination of services plus Trail of Bits devcontainer by adding an explicit guard after reading services and dc_type: if services != "none" and dc_type == "trailofbits" raise/print a clear error and exit (or prompt to remove services) before calling configure_devcontainer_services or setup_devcontainer; this targets the logic around the variables services and dc_type and the calls to configure_devcontainer_services(TEMPLATE_DIR, ...) and setup_devcontainer(TEMPLATE_DIR, devcontainer=...), so the script never reaches the conflicting git clone into .devcontainer when service files already exist.
477-500:⚠️ Potential issue | 🟠 MajorPin the remote devcontainer/firewall sources to immutable revisions.
FIREWALL_GIST_URLpulls whatever the gist's current/raw/content is, andgit clonepulls the current default-branch HEAD from Trail of Bits. That makes setup non-reproducible and lets upstream changes alter generated projects without any change in this repo. Please pin both to a commit/tag and clone that ref explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup_project.py` around lines 477 - 500, The setup_devcontainer function currently uses FIREWALL_GIST_URL and TOB_REPO which point at mutable refs; change both constants to reference immutable revs (a specific commit SHA or tag) and update the clone/fetch logic to use those pinned refs: replace FIREWALL_GIST_URL with the gist raw URL tied to a commit SHA, and when cloning TOB_REPO use an explicit ref (e.g. git clone --branch <tag-or-sha> --single-branch --depth 1 or clone then git checkout <sha>) so the subprocess.run call in setup_devcontainer pins to the specified revision and the fetched firewall script is the exact committed raw URL.
509-513:⚠️ Potential issue | 🟡 MinorUse an
onexccompatibility path for Python 3.12+.
shutil.rmtree(..., onerror=...)is deprecated on the upper end of the supported Python range, so this cleanup path will start emitting deprecation warnings. Please route throughonexcon 3.12+ and keeponerrorfor older interpreters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup_project.py` around lines 509 - 513, The current cleanup uses shutil.rmtree(git_dir, onerror=_remove_readonly) which will emit deprecation warnings on Python 3.12+; update the call to choose the appropriate keyword based on interpreter capability (e.g., detect sys.version_info >= (3,12) or check for an "onexc" parameter in shutil.rmtree signature) and pass the handler as onexc=_remove_readonly for 3.12+ while retaining onerror=_remove_readonly for older Python, keeping the same _remove_readonly function and target git_dir/given call site.
530-537:⚠️ Potential issue | 🔴 CriticalUse a container-visible firewall path and preserve any existing start command.
This block writes
init-firewall.shunder.devcontainer/but injectssudo /usr/local/bin/init-firewall.sh, and the assignment also discards anypostStartCommandalready present in the devcontainer template. Point the command at the downloaded script as seen from inside the container and merge with the existing command instead of replacing it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup_project.py` around lines 530 - 537, The code overwrites any existing postStartCommand and points to /usr/local/bin which isn't the downloaded script location; update the logic around dc_json/config/postStartCommand to construct a container-visible path to the script inside the devcontainer (e.g. the .devcontainer init-firewall.sh location) instead of /usr/local/bin, and merge with any existing postStartCommand (append with "&&" or similar) rather than replacing it so both the template’s command and the firewall init run; use dc_dir and config["postStartCommand"] to detect/compose the final command before writing back to dc_json.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@setup_project.py`:
- Around line 591-595: interactive_setup() currently declares return type
dict[str, str] but stores booleans (e.g., config["egress_firewall"]) which
breaks typing; update the typed shape for config by creating a TypedDict (e.g.,
SetupConfig with keys like "devcontainer" and "egress_firewall") or broaden the
return annotation to dict[str, str | bool], then change interactive_setup()'s
return type to that TypedDict or union type and update any usages; apply the
same fix for the other boolean assignments around the second occurrence (lines
referenced as 649-650) so all keys that may be bool are represented in the type.
- Around line 624-627: The CLI currently accepts --egress-firewall via
parser.add_argument but may never call setup_devcontainer() if devcontainer ==
"none", so add an early validation after parsing args (where devcontainer and
egress_firewall are available): if args.egress_firewall and args.devcontainer !=
"trailofbits" (or args.devcontainer == "none") then fail fast with a clear
error/exit (or at minimum print a warning and exit non-zero); also add the same
guard around the branch that conditionally calls setup_devcontainer() (the code
referenced around the other block at lines 751-755) to ensure the firewall
option cannot be silently ignored.
- Around line 525-526: The current try block uses
urllib.request.urlretrieve(FIREWALL_GIST_URL, firewall_path) which can block
indefinitely; replace it by opening the URL with
urllib.request.urlopen(FIREWALL_GIST_URL, timeout=60) (or similar timeout value
consistent with subprocess.run(..., timeout=60)) and stream the response to
firewall_path using a binary write, handling
socket.timeout/urllib.error.URLError exceptions the same way the existing code
handles failures; update the code referencing FIREWALL_GIST_URL and
firewall_path and ensure any temporary file is flushed/closed on error to avoid
partial writes.
---
Duplicate comments:
In `@setup_project.py`:
- Around line 744-756: Prevent the unsupported combination of services plus
Trail of Bits devcontainer by adding an explicit guard after reading services
and dc_type: if services != "none" and dc_type == "trailofbits" raise/print a
clear error and exit (or prompt to remove services) before calling
configure_devcontainer_services or setup_devcontainer; this targets the logic
around the variables services and dc_type and the calls to
configure_devcontainer_services(TEMPLATE_DIR, ...) and
setup_devcontainer(TEMPLATE_DIR, devcontainer=...), so the script never reaches
the conflicting git clone into .devcontainer when service files already exist.
- Around line 477-500: The setup_devcontainer function currently uses
FIREWALL_GIST_URL and TOB_REPO which point at mutable refs; change both
constants to reference immutable revs (a specific commit SHA or tag) and update
the clone/fetch logic to use those pinned refs: replace FIREWALL_GIST_URL with
the gist raw URL tied to a commit SHA, and when cloning TOB_REPO use an explicit
ref (e.g. git clone --branch <tag-or-sha> --single-branch --depth 1 or clone
then git checkout <sha>) so the subprocess.run call in setup_devcontainer pins
to the specified revision and the fetched firewall script is the exact committed
raw URL.
- Around line 509-513: The current cleanup uses shutil.rmtree(git_dir,
onerror=_remove_readonly) which will emit deprecation warnings on Python 3.12+;
update the call to choose the appropriate keyword based on interpreter
capability (e.g., detect sys.version_info >= (3,12) or check for an "onexc"
parameter in shutil.rmtree signature) and pass the handler as
onexc=_remove_readonly for 3.12+ while retaining onerror=_remove_readonly for
older Python, keeping the same _remove_readonly function and target
git_dir/given call site.
- Around line 530-537: The code overwrites any existing postStartCommand and
points to /usr/local/bin which isn't the downloaded script location; update the
logic around dc_json/config/postStartCommand to construct a container-visible
path to the script inside the devcontainer (e.g. the .devcontainer
init-firewall.sh location) instead of /usr/local/bin, and merge with any
existing postStartCommand (append with "&&" or similar) rather than replacing it
so both the template’s command and the firewall init run; use dc_dir and
config["postStartCommand"] to detect/compose the final command before writing
back to dc_json.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5c0d4859-d290-4153-978b-8e3e50366f3d
📒 Files selected for processing (1)
setup_project.py
Comment on lines
+525
to
+526
| try: | ||
| urllib.request.urlretrieve(FIREWALL_GIST_URL, firewall_path) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find setup_project.py and examine it
fd -t f "setup_project.py"Repository: stranma/claude-code-python-template
Length of output: 123
🏁 Script executed:
# Once we have the file, examine the firewall download code
sed -n '510,540p' setup_project.pyRepository: stranma/claude-code-python-template
Length of output: 1680
🏁 Script executed:
# Also search for the clone path mentioned in the review to see if it has timeout handling
rg -A 5 -B 5 "clone" setup_project.py | head -50Repository: stranma/claude-code-python-template
Length of output: 1443
🏁 Script executed:
# Check if there are other urllib calls in the file to see the pattern
rg "urllib" setup_project.py -B 2 -A 2Repository: stranma/claude-code-python-template
Length of output: 480
🏁 Script executed:
# Check Python documentation or usage of urlretrieve with timeout in similar projects
python3 <<'EOF'
import urllib.request
import inspect
# Check urlretrieve signature
sig = inspect.signature(urllib.request.urlretrieve)
print("urlretrieve signature:")
print(sig)
# Check if it accepts timeout
print("\nFull docstring:")
print(urllib.request.urlretrieve.__doc__)
EOFRepository: stranma/claude-code-python-template
Length of output: 767
🏁 Script executed:
# Also check if there are any tests or other files that show timeout patterns
rg -i "timeout" setup_project.py tests/Repository: stranma/claude-code-python-template
Length of output: 675
🏁 Script executed:
# Check how to properly add timeout to urlretrieve - it may use socket.setdefaulttimeout or we need to use urllib.request.urlopen
python3 <<'EOF'
import urllib.request
import socket
# Check if urlretrieve uses socket timeout
print("Testing urlretrieve timeout mechanisms:")
print()
print("Option 1: socket.setdefaulttimeout()")
print(" - Sets global timeout for all socket operations")
print()
print("Option 2: Use urllib.request.urlopen() instead")
# Check urlopen signature
import inspect
sig = inspect.signature(urllib.request.urlopen)
print(" - urlopen signature:", sig)
print(" - urlopen supports timeout parameter directly")
EOFRepository: stranma/claude-code-python-template
Length of output: 445
🏁 Script executed:
# Let me also check the exact context around the firewall download to see error handling
sed -n '520,545p' setup_project.pyRepository: stranma/claude-code-python-template
Length of output: 1299
🏁 Script executed:
# Let me check what other urllib operations exist and see if there's a pattern
rg -B 3 -A 3 "urllib" setup_project.py | grep -A 5 -B 5 "urlopen\|urlretrieve"Repository: stranma/claude-code-python-template
Length of output: 412
🏁 Script executed:
# Also check if requests library is used elsewhere in the project
rg "import requests" setup_project.py tests/Repository: stranma/claude-code-python-template
Length of output: 61
🏁 Script executed:
# Check requirements or dependencies
fd -t f "requirements*.txt|setup.py|pyproject.toml|Pipfile" | head -5Repository: stranma/claude-code-python-template
Length of output: 145
🏁 Script executed:
# Check the main pyproject.toml for dependencies
cat pyproject.toml | head -50Repository: stranma/claude-code-python-template
Length of output: 1096
🏁 Script executed:
# Check if requests is available as a dependency
grep -i "requests" pyproject.tomlRepository: stranma/claude-code-python-template
Length of output: 61
Add a timeout to the firewall download.
The clone operation uses subprocess.run(..., timeout=60) with timeout handling, but urllib.request.urlretrieve(FIREWALL_GIST_URL, firewall_path) here has no timeout and can block indefinitely on a stalled connection. Use urllib.request.urlopen() with an explicit timeout parameter and write the response to firewall_path to prevent the setup from hanging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@setup_project.py` around lines 525 - 526, The current try block uses
urllib.request.urlretrieve(FIREWALL_GIST_URL, firewall_path) which can block
indefinitely; replace it by opening the URL with
urllib.request.urlopen(FIREWALL_GIST_URL, timeout=60) (or similar timeout value
consistent with subprocess.run(..., timeout=60)) and stream the response to
firewall_path using a binary write, handling
socket.timeout/urllib.error.URLError exceptions the same way the existing code
handles failures; update the code referencing FIREWALL_GIST_URL and
firewall_path and ensure any temporary file is flushed/closed on error to avoid
partial writes.
Comment on lines
+591
to
+595
| if config["devcontainer"] != "none": | ||
| fw_choice = get_input("Include egress firewall? (blocks code exfiltration) [y/n]", "y") | ||
| config["egress_firewall"] = fw_choice.lower() in ("y", "yes") | ||
| else: | ||
| config["egress_firewall"] = False |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Give config a typed shape before adding booleans.
interactive_setup() is annotated as returning dict[str, str], but these hunks now store bool values under egress_firewall. That makes the signature inaccurate and weakens downstream type-checking; a small TypedDict (or at least dict[str, str | bool]) would keep the new fields honest. As per coding guidelines, "Use types everywhere possible in Python code".
Also applies to: 649-650
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@setup_project.py` around lines 591 - 595, interactive_setup() currently
declares return type dict[str, str] but stores booleans (e.g.,
config["egress_firewall"]) which breaks typing; update the typed shape for
config by creating a TypedDict (e.g., SetupConfig with keys like "devcontainer"
and "egress_firewall") or broaden the return annotation to dict[str, str |
bool], then change interactive_setup()'s return type to that TypedDict or union
type and update any usages; apply the same fix for the other boolean assignments
around the second occurrence (lines referenced as 649-650) so all keys that may
be bool are represented in the type.
Comment on lines
+624
to
+627
| parser.add_argument( | ||
| "--egress-firewall", | ||
| action="store_true", | ||
| help="Add egress firewall to devcontainer (blocks code exfiltration)", |
There was a problem hiding this comment.
Reject --egress-firewall when no devcontainer is being installed.
CLI mode accepts this flag on its own, but Step 5b only runs setup_devcontainer() when devcontainer != "none", so the command can finish successfully without installing the firewall the user explicitly asked for. Please fail fast or warn when --egress-firewall is supplied without --devcontainer trailofbits.
Possible guard
if not config.get("project_name"):
print("Error: Project name is required.")
sys.exit(1)
+
+ if config.get("egress_firewall") and config.get("devcontainer", "none") == "none":
+ print("Error: --egress-firewall requires --devcontainer trailofbits.")
+ sys.exit(1)Also applies to: 751-755
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@setup_project.py` around lines 624 - 627, The CLI currently accepts
--egress-firewall via parser.add_argument but may never call
setup_devcontainer() if devcontainer == "none", so add an early validation after
parsing args (where devcontainer and egress_firewall are available): if
args.egress_firewall and args.devcontainer != "trailofbits" (or
args.devcontainer == "none") then fail fast with a clear error/exit (or at
minimum print a warning and exit non-zero); also add the same guard around the
branch that conditionally calls setup_devcontainer() (the code referenced around
the other block at lines 751-755) to ensure the firewall option cannot be
silently ignored.
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.
Summary
.claude/to pyclaude-forge (now on PyPI).devcontainer/to claude-code-devcontainer (now deprecated, users directed to trailofbits/claude-code-devcontainer)--devcontainer trailofbitsand--egress-firewalloptions tosetup_project.pythat compose external componentsComposition
Test plan
python -m pytest tests/)setup_project.py --devcontainer trailofbits --egress-firewall+pyclaude-forge install→ all files in place, settings.json conflict correctly detected and skippedSummary by CodeRabbit
Chores
New Features
Documentation