Fix security issues, reduce duplication, add Java tests, and reconcile agent instructions#36
Open
aseembits93 wants to merge 1 commit intomainfrom
Open
Fix security issues, reduce duplication, add Java tests, and reconcile agent instructions#36aseembits93 wants to merge 1 commit intomainfrom
aseembits93 wants to merge 1 commit intomainfrom
Conversation
…e agent instructions - Security: disable debug logging to /tmp by default (opt-in via CODEFLASH_HOOK_DEBUG=1) - Security: fix shell-to-Python injection patterns in oauth-login.sh (use sys.argv instead of string interpolation) - Refactor: extract append_auto_allow_msg() and emit_block() helpers to eliminate 10 copies of auto-allow message logic - Refactor: rename misleading CHANGED_COMMITS variable to CHANGED_FILES - Fix: quote paths in cd command strings to handle spaces - Tests: add Java project test coverage (7 new test cases + helpers) - Docs: reconcile contradictory JS/TS setup instructions between hook and agent (both now auto-discover) - Housekeeping: add .idea/ and .claude/settings.local.json to .gitignore
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
Addresses multiple issues identified in a code review of the plugin:
Security Fixes
set -xwas unconditionally writing all execution traces (including env vars and API key checks) to a world-readable/tmp/codeflash-hook-debug.log. Now opt-in viaCODEFLASH_HOOK_DEBUG=1.json.load(open('${STATE_FILE}'))) now usesys.argvinstead, preventing breakage or injection from paths containing quotes.Refactoring
append_auto_allow_msg()andemit_block()helpers: Eliminates 10 near-identical copies of the auto-allow message appending and JSON emission pattern throughoutsuggest-optimize.sh.CHANGED_COMMITS→CHANGED_FILES: The variable holds file paths from--name-only, not commit hashes — the old name was misleading.cdcommand strings:RUN_CMD="cd $PROJECT_DIR && ..."now quotes$PROJECT_DIRto handle paths with spaces.Testing
add_java_commit(),create_codeflash_toml(), andsetup_mock_codeflash_bin()test helpers.Documentation
optimizer.mdStep 3b said "ask the user two questions". Both now consistently auto-discover using Glob and Read.Housekeeping
.idea/and.claude/settings.local.jsonto.gitignore.All 38 tests pass (7 find-venv + 31 suggest-optimize).