fix: address 7 security and reliability issues
- Prevent path traversal in gitignore template names (Critical) - Fix TOCTOU races in file operations with try/except pattern (High) - Add timeout to HuggingFace API requests (Medium) - Add missing %x00 separator in changelog fallback format (Medium) - Filter unsanitized kwargs before passing to OpenAI SDK (High) - Use context managers for proper client cleanup in providers (High) - Exit with non-zero code on commit engine failure (Low)
This commit is contained in:
8 files changed
+59
-39
No files matched your search
@@ -58,7 +58,7 @@ class ChangelogGenerator:
|
||||
return self._log_cmd(f"{last_tag}..HEAD")
|
||||
except (RuntimeError, subprocess.CalledProcessError):
|
||||
self.logger.info("No tags found, generating for all commits.")
|
||||
return ["git", "log", "--format=%H|%an|%ad|%s|%b", "--date=short"]
|
||||
return ["git", "log", "--format=%H|%an|%ad|%s|%b%x00", "--date=short"]
|
||||
|
||||
# ----------------------------------------------------------------- parsing
|
||||
|
||||
|
||||
@@ -388,6 +388,7 @@ def run_commit_engine(**kwargs: Any) -> None:
|
||||
sys.exit(0)
|
||||
except Exception as e:
|
||||
logger.error(f"Commit engine failed: {e}", exc_info=True)
|
||||
sys.exit(1)
|
||||
|
||||
|
||||
__all__ = ["CommitEngine", "CommitEngineError", "run_commit_engine"]
|
||||
@@ -1,3 +1,4 @@
|
||||
import re
|
||||
from pathlib import Path
|
||||
from typing import List
|
||||
|
||||
@@ -5,6 +6,9 @@ import requests
|
||||
|
||||
from devgen.utils import configure_logger
|
||||
|
||||
# Only allow safe characters in template names to prevent path traversal
|
||||
_TEMPLATE_NAME_RE = re.compile(r"^[a-zA-Z0-9._-]+$")
|
||||
|
||||
|
||||
class GitignoreGenerator:
|
||||
"""Fetches and manages .gitignore templates from GitHub."""
|
||||
@@ -41,6 +45,11 @@ class GitignoreGenerator:
|
||||
self, name: str, use_cache: bool = True, offline: bool = False
|
||||
) -> str:
|
||||
"""Fetches template content from cache or GitHub."""
|
||||
if not _TEMPLATE_NAME_RE.match(name):
|
||||
raise ValueError(
|
||||
f"Invalid template name '{name}'. "
|
||||
"Names must contain only alphanumeric characters, hyphens, dots, or underscores."
|
||||
)
|
||||
cache_file = self.cache_dir / f"{name}.gitignore"
|
||||
|
||||
if offline:
|
||||
|
||||
@@ -20,7 +20,7 @@ class HuggingfaceProvider(BaseProvider):
|
||||
}
|
||||
|
||||
try:
|
||||
response = requests.post(api_url, headers=headers, json=payload)
|
||||
response = requests.post(api_url, headers=headers, json=payload, timeout=120)
|
||||
response.raise_for_status()
|
||||
result = response.json()
|
||||
except requests.HTTPError as e:
|
||||
|
||||
@@ -18,18 +18,22 @@ class OllamaProvider(BaseProvider):
|
||||
DEFAULT_HOST = "http://localhost:11434"
|
||||
DEFAULT_MODEL = "llama3.2"
|
||||
|
||||
# Keys consumed by devgen itself — never forwarded to the SDK.
|
||||
_DEVGEN_KEYS = frozenset({"debug", "ollama_host", "max_retries", "retry_delay"})
|
||||
|
||||
def _generate(self, prompt, api_key, model, **kwargs):
|
||||
host = kwargs.get("ollama_host") or self.DEFAULT_HOST
|
||||
base_url = f"{host.rstrip('/')}/v1"
|
||||
kwargs.pop("debug", None)
|
||||
kwargs.pop("ollama_host", None)
|
||||
|
||||
client = OpenAI(base_url=base_url, api_key=api_key or "ollama")
|
||||
response = client.chat.completions.create(
|
||||
model=model,
|
||||
messages=[{"role": "user", "content": prompt}],
|
||||
**kwargs,
|
||||
)
|
||||
# Only forward known OpenAI-compatible parameters
|
||||
safe_kwargs = {k: v for k, v in kwargs.items() if k not in self._DEVGEN_KEYS}
|
||||
|
||||
with OpenAI(base_url=base_url, api_key=api_key or "ollama") as client:
|
||||
response = client.chat.completions.create(
|
||||
model=model,
|
||||
messages=[{"role": "user", "content": prompt}],
|
||||
**safe_kwargs,
|
||||
)
|
||||
|
||||
if not response.choices:
|
||||
raise RuntimeError(
|
||||
|
||||
@@ -9,14 +9,17 @@ class OpenaiProvider(BaseProvider):
|
||||
DISPLAY_NAME = "OpenAI"
|
||||
DEFAULT_MODEL = "gpt-4o"
|
||||
|
||||
# Keys consumed by devgen itself — never forwarded to the SDK.
|
||||
_DEVGEN_KEYS = frozenset({"debug", "ollama_host", "max_retries", "retry_delay"})
|
||||
|
||||
def _generate(self, prompt, api_key, model, **kwargs):
|
||||
kwargs.pop("debug", None)
|
||||
client = OpenAI(api_key=api_key)
|
||||
response = client.chat.completions.create(
|
||||
model=model,
|
||||
messages=[{"role": "user", "content": prompt}],
|
||||
**kwargs,
|
||||
)
|
||||
safe_kwargs = {k: v for k, v in kwargs.items() if k not in self._DEVGEN_KEYS}
|
||||
with OpenAI(api_key=api_key) as client:
|
||||
response = client.chat.completions.create(
|
||||
model=model,
|
||||
messages=[{"role": "user", "content": prompt}],
|
||||
**safe_kwargs,
|
||||
)
|
||||
if not response.choices:
|
||||
raise RuntimeError(f"OpenAI returned no choices for model {model!r}.")
|
||||
return response.choices[0].message.content or ""
|
||||
@@ -10,18 +10,21 @@ class OpenrouterProvider(BaseProvider):
|
||||
BASE_URL = "https://openrouter.ai/api/v1"
|
||||
DEFAULT_MODEL = "openai/gpt-3.5-turbo"
|
||||
|
||||
# Keys consumed by devgen itself — never forwarded to the SDK.
|
||||
_DEVGEN_KEYS = frozenset({"debug", "ollama_host", "max_retries", "retry_delay"})
|
||||
|
||||
def _generate(self, prompt, api_key, model, **kwargs):
|
||||
kwargs.pop("debug", None)
|
||||
client = OpenAI(base_url=self.BASE_URL, api_key=api_key)
|
||||
response = client.chat.completions.create(
|
||||
model=model,
|
||||
messages=[{"role": "user", "content": prompt}],
|
||||
extra_headers={
|
||||
"HTTP-Referer": "https://github.com/S4NKALP/devgen",
|
||||
"X-Title": "devgen CLI",
|
||||
},
|
||||
**kwargs,
|
||||
)
|
||||
safe_kwargs = {k: v for k, v in kwargs.items() if k not in self._DEVGEN_KEYS}
|
||||
with OpenAI(base_url=self.BASE_URL, api_key=api_key) as client:
|
||||
response = client.chat.completions.create(
|
||||
model=model,
|
||||
messages=[{"role": "user", "content": prompt}],
|
||||
extra_headers={
|
||||
"HTTP-Referer": "https://github.com/S4NKALP/devgen",
|
||||
"X-Title": "devgen CLI",
|
||||
},
|
||||
**safe_kwargs,
|
||||
)
|
||||
if not response.choices:
|
||||
raise RuntimeError(f"OpenRouter returned no choices for model {model!r}.")
|
||||
return response.choices[0].message.content or ""
|
||||
+11
-11
@@ -24,10 +24,10 @@ def get_commit_dry_run_path() -> Path:
|
||||
|
||||
|
||||
def is_file_recent(file_path: Path | str, max_age_minutes: int = 120) -> bool:
|
||||
path = Path(file_path)
|
||||
if not path.exists():
|
||||
try:
|
||||
return (time.time() - Path(file_path).stat().st_mtime) <= max_age_minutes * 60
|
||||
except FileNotFoundError:
|
||||
return False
|
||||
return (time.time() - path.stat().st_mtime) <= max_age_minutes * 60
|
||||
|
||||
|
||||
def sanitize_ai_commit_message(raw_text: str) -> str:
|
||||
@@ -193,18 +193,18 @@ def get_git_staged_files() -> list[str]:
|
||||
|
||||
|
||||
def read_file_content(filepath: Path | str) -> Optional[str]:
|
||||
path = Path(filepath)
|
||||
if path.exists():
|
||||
return path.read_text(encoding="utf-8")
|
||||
return None
|
||||
try:
|
||||
return Path(filepath).read_text(encoding="utf-8")
|
||||
except FileNotFoundError:
|
||||
return None
|
||||
|
||||
|
||||
def delete_file(filepath: Path | str) -> bool:
|
||||
path = Path(filepath)
|
||||
if path.exists():
|
||||
path.unlink()
|
||||
try:
|
||||
Path(filepath).unlink()
|
||||
return True
|
||||
return False
|
||||
except FileNotFoundError:
|
||||
return False
|
||||
|
||||
|
||||
def load_template_env(sub_dir: str) -> Environment:
|
||||
|
||||
Reference in new issue
Block a user