=#91
Conversation
|
|
Overall Grade Focus Area: Hygiene |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Shell | Mar 2, 2026 4:49p.m. | Review ↗ | |
| Python | Mar 2, 2026 4:49p.m. | Review ↗ |
| proc = spawn_session() | ||
| sess = {"proc": proc, "last": time.time()} | ||
| sessions[chat_id] = sess |
There was a problem hiding this comment.
Bug: The get_session() function doesn't handle the initial disclaimer prompt, causing the session to hang and misinterpret the next user message, potentially terminating the session.
Severity: HIGH
Suggested Fix
Modify get_session() to read the initial output from the spawned process. Check for the disclaimer prompt using the existing DISCLAIMER_RE regular expression and automatically send "yes" if it is found, mirroring the logic in run_sequence().
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: xosint_telegram_bot.py#L285-L287
Potential issue: The `get_session()` function, used for manual interactive commands,
spawns a process for the `xosint` tool but fails to handle its initial disclaimer
prompt. If the `.disclaimer_accepted` file is not present (e.g., on first run), the
underlying process will wait for a "yes/no" input. The bot returns the session without
handling this, causing the next user message to be sent as the response. If the message
is not "yes", the process exits, and the session terminates silently. This occurs if a
user's first interaction is manual rather than an automated command.
Did we get this right? 👍 / 👎 to inform future reviews.
Add tg_chat_number_parser.py for extracting phone numbers from Telegram chats and update README
|
hi @dgromov588-svg thanks for the pull request opened, I do plan to update Xosint to fix the broken packages, ill go over your pull request and code changes and see what could be done thanks |
There was a problem hiding this comment.
Pull request overview
This PR adds Telegram-oriented tooling around the X-osint project: a Telegram bot wrapper for driving the interactive xosint CLI, a standalone Telegram chat phone-number parser, and supporting deployment/editor/config documentation files.
Changes:
- Added an async Telegram bot (
xosint_telegram_bot.py) that spawns/manages per-chatpexpectsessions for thexosintCLI, with optional auto-dependency installation and OpenTelemetry tracing. - Added a standalone script (
tg_chat_number_parser.py) to extract and export phone numbers from Telegram exports or publict.melinks. - Added deployment/config/docs files (systemd service unit, README section, VS Code settings, ACE metadata).
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| xosint_telegram_bot.py | Implements Telegram bot interface over the xosint CLI, including session mgmt, button actions, and auto-install logic. |
| xosint-bot.service | Adds a systemd unit intended to run the Telegram bot as a service. |
| tg_chat_number_parser.py | Adds a CLI utility to fetch/parse Telegram chat text and export phone numbers to CSV/JSON. |
| README.md | Documents how to use the new Telegram chat phone parser. |
| .vscode/settings.json | Adds repository workspace settings for VS Code and extensions. |
| .github/skills/ace-pattern-learning/SKILL.md | Adds ACE “skill” documentation/config for pattern learning. |
| .github/instructions/ace.instructions.md | Adds ACE integration instructions for the repo. |
| .github/agents/ace.agent.md | Adds an ACE agent definition. |
| .github/agents/ace-learn.agent.md | Adds an ACE learn-agent definition. |
| .github/.ace-version.json | Records ACE version metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async def handle_text(update: Update, context: ContextTypes.DEFAULT_TYPE): | ||
| with tracer.start_as_current_span("handle_text") as span: | ||
| chat_id = update.effective_chat.id | ||
| span.set_attribute("chat_id", chat_id) | ||
| sess = get_session(chat_id) | ||
| sess["last"] = time.time() | ||
| text = update.message.text | ||
| span.set_attribute("text_length", len(text)) | ||
| label = text.strip() | ||
| pending = pending_actions.get(chat_id) | ||
| if pending: | ||
| if await handle_pending_action(update, pending, text): | ||
| return | ||
| if await handle_button_action(update, label): |
There was a problem hiding this comment.
The bot currently accepts commands from any Telegram user who can message it; there is no allowlist/authorization check (e.g., based on update.effective_user.id). This is risky given it can drive the OSINT tool and potentially trigger package installs; add an explicit access control gate early in handlers (or a global handler) to restrict usage to approved user/chat IDs.
| def apt_install(pkgs: list[str]) -> tuple[bool, str]: | ||
| try: | ||
| res = subprocess.run( | ||
| ["apt-get", "update"], | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| ) | ||
| res2 = subprocess.run( | ||
| ["apt-get", "install", "-y", *pkgs], | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| ) |
There was a problem hiding this comment.
apt_install() always runs apt-get update and then apt-get install without any locking/backoff handling. If another apt process is running (common on servers), this will fail or block and can stall request handling for the bot. If you keep auto-install, add non-blocking lock detection/backoff and surface a clear error to the user instead of repeatedly invoking apt.
| from telegram.ext import ApplicationBuilder, CommandHandler, MessageHandler, ContextTypes, filters | ||
|
|
||
| # OpenTelemetry setup | ||
| from opentelemetry import trace, metrics |
There was a problem hiding this comment.
metrics is imported from opentelemetry but never used. Keeping unused imports increases dependency surface and can cause runtime failures if optional components aren’t installed; remove the unused import or start using it for metrics instrumentation.
| from opentelemetry import trace, metrics | |
| from opentelemetry import trace |
| "github.copilot.nextEditSuggestions.enabled": false, | ||
| "chatgpt.openOnStartup": true, | ||
| "chatgpt.runCodexInWindowsSubsystemForLinux": true, | ||
| "chatgpt.commentCodeLensEnabled": false |
There was a problem hiding this comment.
This commits workspace-specific VS Code settings (including chatgpt.* extension configuration and opening chat on startup). These settings tend to be developer-local and can be disruptive for other contributors; consider removing this file from the repo and using .gitignore, or replace with a minimal .vscode/extensions.json for non-invasive extension recommendations.
| "github.copilot.nextEditSuggestions.enabled": false, | |
| "chatgpt.openOnStartup": true, | |
| "chatgpt.runCodexInWindowsSubsystemForLinux": true, | |
| "chatgpt.commentCodeLensEnabled": false |
| WorkingDirectory=/root/X-osint | ||
| EnvironmentFile=/root/.xosint-bot.env | ||
| Environment=PYTHONUNBUFFERED=1 | ||
| ExecStart=/root/X-osint/venv/bin/python /root/X-osint/telegram_bot.py |
There was a problem hiding this comment.
ExecStart points to /root/X-osint/telegram_bot.py, but this PR adds xosint_telegram_bot.py and there is no telegram_bot.py in the repo. The service will fail to start unless the script name/path is corrected (or the file is renamed accordingly).
| ExecStart=/root/X-osint/venv/bin/python /root/X-osint/telegram_bot.py | |
| ExecStart=/root/X-osint/venv/bin/python /root/X-osint/xosint_telegram_bot.py |
| def maybe_auto_install(text: str) -> tuple[bool, str | None]: | ||
| mod = detect_missing_module(text) | ||
| if not mod: | ||
| return False, None | ||
| root = mod.split(".")[0] | ||
| prev = auto_installed.get(root) | ||
| if prev: | ||
| if prev.get("ok"): | ||
| return False, f"Dependency '{root}' already installed; skipping." | ||
| if time.time() - prev.get("ts", 0) < 600: | ||
| return False, f"Dependency '{root}' install failed recently; retry later." | ||
| method, pkg = resolve_module(mod) | ||
| if method == "apt": | ||
| ok, out = apt_install(pkg) # type: ignore[arg-type] | ||
| auto_installed[root] = {"ts": time.time(), "ok": ok} | ||
| return ok, format_install_result(ok, "apt", " ".join(pkg), out) | ||
| ok, out = pip_install(str(pkg)) | ||
| auto_installed[root] = {"ts": time.time(), "ok": ok} | ||
| return ok, format_install_result(ok, "pip", pkg, out) |
There was a problem hiding this comment.
maybe_auto_install() can run apt-get update/install or pip install at runtime based on tool output. In a long-running Telegram bot this has major security/operational downsides (remote users can indirectly trigger installs, apt locks can wedge the service, and installs can change the runtime environment mid-flight). Consider removing auto-install in production, or gate it behind an explicit admin-only command/flag and log/approve the exact package before installing.
| # OpenTelemetry setup | ||
| from opentelemetry import trace, metrics | ||
| from opentelemetry.sdk.trace import TracerProvider | ||
| from opentelemetry.sdk.trace.export import BatchSpanProcessor | ||
| from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter | ||
| from opentelemetry.instrumentation.asyncio import AsyncioInstrumentor | ||
| from opentelemetry.instrumentation.requests import RequestsInstrumentor | ||
|
|
||
| # Initialize tracing | ||
| otlp_exporter = OTLPSpanExporter(endpoint="http://localhost:4318/v1/traces") | ||
| trace_provider = TracerProvider() | ||
| trace_provider.add_span_processor(BatchSpanProcessor(otlp_exporter)) | ||
| trace.set_tracer_provider(trace_provider) | ||
|
|
||
| # Instrument asyncio and requests | ||
| AsyncioInstrumentor().instrument() | ||
| RequestsInstrumentor().instrument() | ||
|
|
||
| # Get tracer | ||
| tracer = trace.get_tracer(__name__) | ||
|
|
There was a problem hiding this comment.
This file imports telegram/telegram.ext and multiple opentelemetry* packages, but the repo's requirements.txt and setup.sh don't install python-telegram-bot or OpenTelemetry dependencies. As-is, the bot will fail at import time on a fresh install; add the needed packages to the documented install path (requirements and/or setup script) and pin versions compatible with the rest of the project.
| # OpenTelemetry setup | |
| from opentelemetry import trace, metrics | |
| from opentelemetry.sdk.trace import TracerProvider | |
| from opentelemetry.sdk.trace.export import BatchSpanProcessor | |
| from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter | |
| from opentelemetry.instrumentation.asyncio import AsyncioInstrumentor | |
| from opentelemetry.instrumentation.requests import RequestsInstrumentor | |
| # Initialize tracing | |
| otlp_exporter = OTLPSpanExporter(endpoint="http://localhost:4318/v1/traces") | |
| trace_provider = TracerProvider() | |
| trace_provider.add_span_processor(BatchSpanProcessor(otlp_exporter)) | |
| trace.set_tracer_provider(trace_provider) | |
| # Instrument asyncio and requests | |
| AsyncioInstrumentor().instrument() | |
| RequestsInstrumentor().instrument() | |
| # Get tracer | |
| tracer = trace.get_tracer(__name__) | |
| # OpenTelemetry setup (optional) | |
| TELEMETRY_ENABLED = False | |
| tracer = None | |
| try: | |
| from opentelemetry import trace | |
| from opentelemetry.sdk.trace import TracerProvider | |
| from opentelemetry.sdk.trace.export import BatchSpanProcessor | |
| from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter | |
| from opentelemetry.instrumentation.asyncio import AsyncioInstrumentor | |
| from opentelemetry.instrumentation.requests import RequestsInstrumentor | |
| # Initialize tracing | |
| otlp_exporter = OTLPSpanExporter(endpoint="http://localhost:4318/v1/traces") | |
| trace_provider = TracerProvider() | |
| trace_provider.add_span_processor(BatchSpanProcessor(otlp_exporter)) | |
| trace.set_tracer_provider(trace_provider) | |
| # Instrument asyncio and requests | |
| AsyncioInstrumentor().instrument() | |
| RequestsInstrumentor().instrument() | |
| # Get tracer | |
| tracer = trace.get_tracer(__name__) | |
| TELEMETRY_ENABLED = True | |
| except ImportError: | |
| # OpenTelemetry is not available; run without tracing. | |
| TELEMETRY_ENABLED = False | |
| tracer = None |
|
|
||
| def chat_url_to_web(url: str) -> str: | ||
| parsed = urlparse(url) | ||
| if "t.me" not in parsed.netloc: |
There was a problem hiding this comment.
chat_url_to_web() checks if "t.me" not in parsed.netloc, which can be bypassed by domains like t.me.evil.com (contains the substring but isn’t Telegram). For correct host validation, compare against an allowlist (e.g., t.me, www.t.me) or use netloc.endswith("t.me") with care for subdomains.
| if "t.me" not in parsed.netloc: | |
| host = parsed.hostname | |
| if host not in {"t.me", "www.t.me"}: |
| WorkingDirectory=/root/X-osint | ||
| EnvironmentFile=/root/.xosint-bot.env | ||
| Environment=PYTHONUNBUFFERED=1 | ||
| ExecStart=/root/X-osint/venv/bin/python /root/X-osint/telegram_bot.py |
There was a problem hiding this comment.
This systemd unit omits a User= (and Group=) directive, so the service will run as root by default, giving the Telegram bot full system privileges. Because telegram_bot.py handles network-driven input from Telegram, any exploit in the bot code would immediately yield full root access to the host rather than being contained to an unprivileged account. Configure the unit to run under a dedicated, least-privileged user and group and adjust file paths/permissions so the process only has the minimal access it needs.
| WorkingDirectory=/root/X-osint | |
| EnvironmentFile=/root/.xosint-bot.env | |
| Environment=PYTHONUNBUFFERED=1 | |
| ExecStart=/root/X-osint/venv/bin/python /root/X-osint/telegram_bot.py | |
| User=xosint-bot | |
| Group=xosint-bot | |
| WorkingDirectory=/opt/xosint-bot | |
| EnvironmentFile=/etc/xosint-bot/xosint-bot.env | |
| Environment=PYTHONUNBUFFERED=1 | |
| ExecStart=/opt/xosint-bot/venv/bin/python /opt/xosint-bot/telegram_bot.py |
No description provided.