Enhance security & Code Quality

.gitignore

@@ -12,3 +12,4 @@ state.json.bak
 logs/
 .codex
 .copilot
+.env

README.md

@@ -258,6 +258,14 @@ GitHub documents these Copilot CLI approval controls here:
   If `true`, enable the `/commit` Telegram command.
   Default: `false`
 
+- `AGENT_HARD_TIMEOUT_SECONDS`
+  Hard time limit in seconds for a single agent run.
+  When the agent subprocess is still running after this many seconds, the bot sends a timeout message and terminates the process.
+  Set to `0` (the default) to disable the limit entirely.
+  Disabling is recommended for Copilot, which can legitimately take over an hour on complex tasks.
+  Set a value (e.g. `600`) only when you want a hard cap — typically for shorter, bounded Codex jobs.
+  Default: `0` (disabled)
+
 ### Telegram Behavior
 
 - `SNAPSHOT_TEXT_FILE_MAX_BYTES`
@@ -357,7 +365,18 @@ Each session stores:
 - timestamps
 - active session selection for that bot/chat scope
 
-## ⚠️ Diff (file changes)
+### Workspace concurrency lock
+
+Only one agent run can be active per **project folder** at a time — regardless of which chat ID or Telegram bot triggers it.
+
+If a message arrives while an agent is already running on the same project, the bot immediately replies:
+
+> ⏳ An agent is already running on project '…'. Please wait for it to finish.
+
+The lock is held in memory (not on disk), so it is automatically released when the agent finishes, errors out, or if the server restarts. There are no stale lock files to clean up after a crash.
+
+## ⚠  Diff (file changes)
+
 _During each agent run, the bot also takes a lightweight before/after project snapshot so it can summarize changed files and send diffs back to Telegram. This snapshot is taken by the bot app itself, not by Codex or Copilot._
 
 **Snapshot notes:**
@@ -418,11 +437,16 @@ The chosen branch is stored with the session, so switching sessions restores the
 
 ## 🪵 Logs
 
-Logs are written under `LOG_DIR`.
+Logs are written to **both stdout and a rotating log file** under `LOG_DIR`.
 
 Main log file:
 
-- `coding-agent-telegram.log`
+- `coding-agent-telegram.log` (rotated at 10 MB, 3 backups kept)
+
+> **Note:** Because messages go to both stdout and the log file, watching the terminal
+> **and** tailing the log file at the same time (e.g. `tail -f logs/coding-agent-telegram.log`)
+> will make each message appear twice — once from each sink. This is expected behavior.
+> View one or the other, not both simultaneously.
 
 Typical logged events:
 
@@ -431,7 +455,7 @@ Typical logged events:
 - session creation
 - session switching
 - active session reporting
-- normal run execution
+- normal run execution (includes an audit log line with the truncated prompt)
 - session replacement after resume failure
 - warnings and runtime errors
 

src/coding_agent_telegram/agent_runner.py

@@ -3,6 +3,7 @@
 import json
 import logging
 import os
+import re
 import subprocess
 import tempfile
 import threading
@@ -16,6 +17,27 @@
 
 AssistantEvent = Union[dict[str, Any], list[Any], str]
 
+# Only allow alphanumeric, hyphens, underscores, and dots — must not start with "-"
+# to prevent a crafted LLM session_id from injecting CLI flags.
+_SESSION_ID_RE = re.compile(r"^[A-Za-z0-9_.\-]{1,128}$")
+
+
+def _validate_session_id(raw: str) -> Optional[str]:
+    """Return ``raw`` only if it looks like a legitimate session identifier.
+
+    Rejects values that could be used to inject CLI flags (e.g. ``--exec``)
+    into subsequent subprocess calls that pass the session ID as an argument.
+    """
+    if not raw or not isinstance(raw, str):
+        return None
+    if raw.startswith("-"):
+        logger.warning("Rejected session_id starting with '-' from agent output: %r", raw[:64])
+        return None
+    if _SESSION_ID_RE.match(raw):
+        return raw
+    logger.warning("Rejected suspicious session_id from agent output: %r", raw[:64])
+    return None
+
 
 @dataclass
 class AgentRunResult:
@@ -69,6 +91,7 @@ def __init__(
         copilot_allow_tools: tuple[str, ...] = (),
         copilot_deny_tools: tuple[str, ...] = (),
         copilot_available_tools: tuple[str, ...] = (),
+        hard_timeout_seconds: int = 0,
     ) -> None:
         self.codex_bin = codex_bin
         self.copilot_bin = copilot_bin
@@ -83,6 +106,8 @@ def __init__(
         self.copilot_allow_tools = tuple(tool.strip() for tool in copilot_allow_tools if tool.strip())
         self.copilot_deny_tools = tuple(tool.strip() for tool in copilot_deny_tools if tool.strip())
         self.copilot_available_tools = tuple(tool.strip() for tool in copilot_available_tools if tool.strip())
+        # 0 = disabled. When > 0, the agent subprocess is killed after this many seconds.
+        self.hard_timeout_seconds = max(0, int(hard_timeout_seconds))
 
     def _looks_textual_key(self, key: str) -> bool:
         normalized = key.strip().lower()
@@ -185,7 +210,9 @@ def _parse_jsonl(
         for ev in events:
             for key in ("session_id", "thread_id", "sessionId", "threadId"):
                 if isinstance(ev.get(key), str):
-                    session_id = ev[key]
+                    validated = _validate_session_id(ev[key])
+                    if validated:
+                        session_id = validated
             extracted_text = assistant_text_extractor(ev)
             if extracted_text:
                 assistant_text = extracted_text
@@ -323,6 +350,27 @@ def read_stream(stream, chunks: list[str], *, is_stderr: bool) -> None:
         stdout_thread.start()
         stderr_thread.start()
 
+        # Watchdog: kills the process if it exceeds the hard timeout.
+        # Uses threading.Event (not time.monotonic or proc.poll) so it does not
+        # interfere with existing stall-detection logic or test mocks.
+        # A timeout of 0 means disabled — the process can run indefinitely.
+        _proc_exited = threading.Event()
+        _watchdog_timeout = self.hard_timeout_seconds if self.hard_timeout_seconds > 0 else None
+
+        def _watchdog() -> None:
+            if not _proc_exited.wait(timeout=_watchdog_timeout):
+                logger.warning(
+                    "Agent command exceeded hard timeout of %ds; terminating process.",
+                    _watchdog_timeout,
+                )
+                try:
+                    proc.kill()
+                except OSError:
+                    pass
+
+        watchdog_thread = threading.Thread(target=_watchdog, daemon=True, name="agent-watchdog")
+        watchdog_thread.start()
+
         stall_reported = False
         while proc.poll() is None:
             if on_stall and not stall_reported:
@@ -353,6 +401,8 @@ def read_stream(stream, chunks: list[str], *, is_stderr: bool) -> None:
 
         stdout_thread.join()
         stderr_thread.join()
+        _proc_exited.set()
+        watchdog_thread.join()
         stdout = "".join(stdout_chunks)
         stderr = "".join(stderr_chunks)
         if provider == "codex":

src/coding_agent_telegram/bot.py

@@ -7,6 +7,7 @@
 from telegram.ext import Application, CallbackQueryHandler, CommandHandler, MessageHandler, filters as tg_filters
 
 from coding_agent_telegram.command_router import CommandRouter
+from coding_agent_telegram.session_store import SessionStoreError
 
 
 logger = logging.getLogger(__name__)
@@ -45,6 +46,14 @@ async def initialize_bot_commands(app: Application, *, enable_commit_command: bo
 
 
 async def handle_error(update, context) -> None:
+    if isinstance(context.error, SessionStoreError):
+        logger.warning("Session store lock conflict: %s", context.error)
+        if update is not None and getattr(update, "effective_chat", None) is not None:
+            await context.bot.send_message(
+                chat_id=update.effective_chat.id,
+                text=f"⚠️ {context.error}",
+            )
+        return
     logger.exception("Telegram handler failed.", exc_info=context.error)
     if update is not None and getattr(update, "effective_chat", None) is not None:
         await context.bot.send_message(

src/coding_agent_telegram/cli.py

@@ -125,6 +125,7 @@ def main() -> None:
         copilot_allow_tools=cfg.copilot_allow_tools,
         copilot_deny_tools=cfg.copilot_deny_tools,
         copilot_available_tools=cfg.copilot_available_tools,
+        hard_timeout_seconds=cfg.agent_hard_timeout_seconds,
     )
     try:
         asyncio.run(_run(cfg, store, runner))

src/coding_agent_telegram/config.py

@@ -14,6 +14,8 @@
 DEFAULT_MAX_PHOTO_ATTACHMENT_BYTES = 5 * 1024 * 1024
 DEFAULT_ENV_FILE_NAME = ".env_coding_agent_telegram"
 LEGACY_ENV_FILE_NAME = ".env"
+# 0 = disabled. Set to a positive value to kill runaway agent processes.
+DEFAULT_AGENT_HARD_TIMEOUT_SECONDS = 0
 
 
 @dataclass(frozen=True)
@@ -44,6 +46,7 @@ class AppConfig:
     max_telegram_message_length: int
     enable_sensitive_diff_filter: bool
     default_agent_provider: str
+    agent_hard_timeout_seconds: int
 
 
 def _parse_bool(value: str, default: bool = False) -> bool:
@@ -141,4 +144,7 @@ def load_config(env_file: Optional[Path] = None) -> AppConfig:
         ),
         enable_sensitive_diff_filter=_parse_bool(os.getenv("ENABLE_SENSITIVE_DIFF_FILTER", "true"), default=True),
         default_agent_provider=provider,
+        agent_hard_timeout_seconds=int(
+            os.getenv("AGENT_HARD_TIMEOUT_SECONDS", str(DEFAULT_AGENT_HARD_TIMEOUT_SECONDS))
+        ),
     )

src/coding_agent_telegram/diff_utils.py

@@ -3,6 +3,7 @@
 import difflib
 import fnmatch
 import importlib.resources
+import logging
 import os
 import subprocess
 from functools import lru_cache
@@ -15,6 +16,8 @@
 INTERNAL_APP_DIR = ".coding-agent-telegram"
 TEXTUAL_DIFF_UNAVAILABLE = "Binary or large file changed; textual diff unavailable."
 
+logger = logging.getLogger(__name__)
+
 
 @dataclass
 class FileDiff:
@@ -116,7 +119,8 @@ def is_snapshot_excluded_path(path: str) -> bool:
 def _read_snapshot_text(file_path: Path, *, max_text_file_bytes: int) -> Optional[str]:
     try:
         data = file_path.read_bytes()
-    except OSError:
+    except OSError as exc:
+        logger.debug("Could not read snapshot file %s: %s", file_path, exc)
         return None
     if len(data) > max_text_file_bytes:
         return None

src/coding_agent_telegram/git_utils.py

@@ -10,15 +10,7 @@
 
 
 def _sanitize_git_output(text: str) -> str:
-    """
-    Sanitize git command output to remove credential information.
-
-    Removes:
-    - HTTPS URLs with embedded credentials: https://user:password@
-    - SSH connection strings that might contain sensitive info
-
-    Security-critical function: Prevents credential leaks in logs.
-    """
+    """Sanitize git command output to remove credential information."""
     if not text:
         return text
 
@@ -37,6 +29,20 @@ def _sanitize_git_output(text: str) -> str:
     return sanitized
 
 
+# Allows letters, digits, dots, hyphens, forward-slashes (remote/branch).
+# Must not start with '-' to prevent flag injection into git subcommands.
+_BRANCH_NAME_RE = re.compile(r"^[A-Za-z0-9._/\-]{1,200}$")
+
+
+def _validate_branch_name(name: str) -> bool:
+    """Return True only if *name* is safe to pass as a git branch name argument."""
+    if not name:
+        return False
+    if name.startswith("-"):
+        return False
+    return bool(_BRANCH_NAME_RE.match(name))
+
+
 @dataclass
 class GitCommandResult:
     success: bool
@@ -157,6 +163,8 @@ def refresh_current_branch(self, project_path: Path) -> BranchOperationResult:
         )
 
     def checkout_branch(self, project_path: Path, branch_name: str) -> BranchOperationResult:
+        if not _validate_branch_name(branch_name):
+            return BranchOperationResult(False, f"Invalid branch name: {branch_name!r}")
         result = self._run(project_path, "checkout", branch_name)
         if result.returncode != 0:
             return BranchOperationResult(False, _sanitize_git_output(result.stderr.strip()) or f"Failed to checkout branch: {branch_name}")
@@ -171,6 +179,10 @@ def prepare_branch(
     ) -> BranchOperationResult:
         if not self.is_git_repo(project_path):
             return BranchOperationResult(False, "Current project is not a git repository.")
+        if not _validate_branch_name(new_branch):
+            return BranchOperationResult(False, f"Invalid branch name: {new_branch!r}")
+        if origin_branch and not _validate_branch_name(origin_branch):
+            return BranchOperationResult(False, f"Invalid origin branch name: {origin_branch!r}")
 
         current_branch = self.current_branch(project_path)
         default_branch = self.default_branch(project_path)

src/coding_agent_telegram/logging_utils.py

@@ -1,17 +1,32 @@
 import logging
 import sys
+from logging.handlers import RotatingFileHandler
 from pathlib import Path
 
 
 def setup_logging(level: str, log_dir: Path) -> Path:
     log_dir.mkdir(parents=True, exist_ok=True)
     log_file = log_dir / "coding-agent-telegram.log"
-    logging.basicConfig(
-        level=getattr(logging, level.upper(), logging.INFO),
-        format="%(asctime)s %(levelname)s %(name)s: %(message)s",
-        handlers=[logging.StreamHandler(sys.stdout)],
-        force=True,
+    log_level = getattr(logging, level.upper(), logging.INFO)
+    fmt = logging.Formatter("%(asctime)s %(levelname)s %(name)s: %(message)s")
+
+    root = logging.getLogger()
+    root.setLevel(log_level)
+    root.handlers.clear()
+
+    stdout_handler = logging.StreamHandler(sys.stdout)
+    stdout_handler.setFormatter(fmt)
+    root.addHandler(stdout_handler)
+
+    file_handler = RotatingFileHandler(
+        log_file,
+        maxBytes=10 * 1024 * 1024,
+        backupCount=3,
+        encoding="utf-8",
     )
+    file_handler.setFormatter(fmt)
+    root.addHandler(file_handler)
+
     for logger_name in ("httpx", "httpcore", "telegram", "telegram.ext.ExtBot"):
         logging.getLogger(logger_name).setLevel(logging.WARNING)
     return log_file

src/coding_agent_telegram/resources/.env.example

@@ -91,3 +91,12 @@ ENABLE_SENSITIVE_DIFF_FILTER=true
 
 # Default agent provider for new sessions: codex or copilot.
 DEFAULT_AGENT_PROVIDER=codex
+
+# Hard time limit (seconds) for a single agent run.
+# When the subprocess is still running after this many seconds the bot sends
+# a timeout message and terminates the agent.
+# Set to 0 (the default) to disable the limit entirely — recommended when
+# using Copilot, which can legitimately take over an hour for heavy tasks.
+# Useful as a safeguard when using Codex for shorter, bounded jobs.
+# Example: AGENT_HARD_TIMEOUT_SECONDS=600  (10 minutes)
+AGENT_HARD_TIMEOUT_SECONDS=0