Repository navigation
fix(adhoc-sweep-fixes): CU-17tkuw5uezv 2 review findings across 2 files - #117
flamingo[bot] wants to merge 2 commits into
Conversation
![flamingo[bot]](https://avatars.githubusercontent.com/in/3426709?s=60&v=4){kind=small}
| "Or in config file: main_base_url = '<url>'" | ||
| ) | ||
|
|
||
| # Prepare default headers for API version (Anthropic models) |
There was a problem hiding this comment.
🦩 🟠 create_main_model / create_fallback_model build unused default_headers dict and never attach it to the provider
In create_main_model, create_fallback_model, and create_cluster_model the previously-dead default_headers dict is now actually used: when *_api_version is set, an AsyncOpenAI client is constructed with default_headers={'anthropic-version': api_version} and passed to OpenAIProvider(openai_client=...), per the comment that was already in the code; otherwise the provider is built with base_url=/api_key= as before. Added AsyncOpenAI to the existing from openai import ... line (module already exists as a dependency, no new import added). Unverified: exact OpenAIProvider(openai_client=...) signature/behavior across pydantic-ai versions beyond what the existing comment asserted; if openai_client= also requires/forbids other params in some pydantic-ai release, this could need adjustment, but it directly implements the fix the comment itself specified.
🤖 Prompt for AI agents
In codewiki/src/be/llm_services.py around line 118, review and complete this code-review fix: create_main_model / create_fallback_model build unused default_headers dict and never attach it to the provider.
What the draft fix changed: In `create_main_model`, `create_fallback_model`, and `create_cluster_model` the previously-dead `default_headers` dict is now actually used: when `*_api_version` is set, an `AsyncOpenAI` client is constructed with `default_headers={'anthropic-version': api_version}` and passed to `OpenAIProvider(openai_client=...)`, per the comment that was already in the code; otherwise the provider is built with `base_url=`/`api_key=` as before. Added `AsyncOpenAI` to the existing `from openai import ...` line (module already exists as a dependency, no new import added). Unverified: exact `OpenAIProvider(openai_client=...)` signature/behavior across pydantic-ai versions beyond what the existing comment asserted; if `openai_client=` also requires/forbids other params in some pydantic-ai release, this could need adjustment, but it directly implements the fix the comment itself specified.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 78 medium — react 👍/👎 to teach the reviewer
| """Run flake8 on a given file and return the output as a string""" | ||
| if Path(file_path).suffix != ".py": | ||
| return "" | ||
| cmd = "flake8 --isolated --select=F821,F822,F831,E111,E112,E113,E999,E902 {file_path}" | ||
| cmd = ["flake8", "--isolated", "--select=F821,F822,F831,E111,E112,E113,E999,E902", file_path] | ||
| # don't use capture_output because it's not compatible with python3.6 | ||
| out = subprocess.run(cmd.format(file_path=file_path), shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) | ||
| out = subprocess.run(cmd, shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE) | ||
| return out.stdout.decode() | ||
|
|
||
|
|
There was a problem hiding this comment.
🦩 🟠 flake8() invokes subprocess.run with shell=True and unsanitized file_path interpolation
In flake8(), replaced the shell-interpolated string command (cmd.format(file_path=file_path) run with shell=True) with a list-of-args command (["flake8", "--isolated", "--select=...", file_path]) run with shell=False. This eliminates the command injection vector since file_path is now passed as a single discrete argv element rather than being interpolated into a shell-parsed string, matching the suggested fix exactly. The unrelated view() method's subprocess.run(..., shell=True) for find was left untouched since it was not part of this finding's evidence/scope.
🤖 Prompt for AI agents
In codewiki/src/be/agent_tools/str_replace_editor.py around line 187, review and complete this code-review fix: flake8() invokes subprocess.run with shell=True and unsanitized file_path interpolation.
What the draft fix changed: In `flake8()`, replaced the shell-interpolated string command (`cmd.format(file_path=file_path)` run with `shell=True`) with a list-of-args command (`["flake8", "--isolated", "--select=...", file_path]`) run with `shell=False`. This eliminates the command injection vector since `file_path` is now passed as a single discrete argv element rather than being interpolated into a shell-parsed string, matching the suggested fix exactly. The unrelated `view()` method's `subprocess.run(..., shell=True)` for `find` was left untouched since it was not part of this finding's evidence/scope.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 97 high — react 👍/👎 to teach the reviewer
flamingo
Bot
changed the title
Closes 2 review findings across 2 files.
Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
codewiki/src/be/llm_services.py:118codewiki/src/be/agent_tools/str_replace_editor.py:187What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
c0f6ce38-9374-4249-ab94-7ff9724ef5e6Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.
ClickUp task: CU-17tkuw5uezv CodeWiki review findings sweep (1 PRs)