Skip to content

Feature/neurocore integration#41

Merged
Neuro-Rift merged 16 commits into
mainfrom
feature/neurocore-integration
Mar 22, 2026
Merged

Feature/neurocore integration#41
Neuro-Rift merged 16 commits into
mainfrom
feature/neurocore-integration

Conversation

@Neuro-Rift
Copy link
Copy Markdown
Owner

@Neuro-Rift Neuro-Rift commented Mar 22, 2026

CodeAnt-AI Description

Switch the AI pipeline to llama.cpp and add new agent/runtime workflows

What Changed

  • Replaced Ollama-based AI handling with llama.cpp across the app, including model checks, local generation, and the configuration wizard.
  • Added a new autonomous agent path with startup checks for the runtime and model capabilities before running.
  • Updated CVE collection, recon, exploit, web, dark web, session, and reporting flows to use the new local AI client and keep their outputs working with the new setup.
  • Added skill installation and execution commands, plus a new top-level entry point for the refreshed CLI.

Impact

✅ Runs AI features against the new local model server
✅ Fewer startup failures when the AI service is missing or misconfigured
✅ Faster access to agent and skill workflows from the command line

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Mar 22, 2026

CodeAnt AI is reviewing your PR.


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 22, 2026

Important

Review skipped

Too many files!

This PR contains 270 files, which is 120 over the limit of 150.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1be4bdf8-b4a1-4d28-8606-46f4a6e6b1bb

📥 Commits

Reviewing files that changed from the base of the PR and between 334b4f0 and ac1a8d8.

⛔ Files ignored due to path filters (1)
  • recon/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (270)
  • .gitignore
  • .planning/PROJECT.md
  • .planning/REQUIREMENTS.md
  • .planning/ROADMAP.md
  • .planning/STATE.md
  • .planning/codebase/ARCHITECTURE.md
  • .planning/codebase/CONCERNS.md
  • .planning/codebase/CONVENTIONS.md
  • .planning/codebase/INTEGRATIONS.md
  • .planning/codebase/STACK.md
  • .planning/codebase/STRUCTURE.md
  • .planning/codebase/TESTING.md
  • .planning/config.json
  • .planning/quick/260322-hnv-replace-all-ollama-integration-in-this-c/260322-hnv-PLAN.md
  • .planning/quick/260322-hnv-replace-all-ollama-integration-in-this-c/260322-hnv-RESEARCH.md
  • .planning/quick/260322-icf-full-feature-test-bug-hunt-auto-fix-for-/260322-icf-PLAN.md
  • .planning/quick/260322-icf-full-feature-test-bug-hunt-auto-fix-for-/260322-icf-RESEARCH.md
  • .planning/quick/260322-iwr-neurorift-v2-multi-language-architecture/260322-iwr-PLAN.md
  • .planning/quick/260322-iwr-neurorift-v2-multi-language-architecture/260322-iwr-RESEARCH.md
  • BUG_HUNT_REPORT.md
  • DOCKER.md
  • Makefile
  • NeuroCore
  • README.md
  • ai/__init__.py
  • ai/executor.py
  • ai/llama_client.py
  • ai/planner.py
  • ai/prompts/executor_system.txt
  • ai/prompts/planner_system.txt
  • ai_wrapper/__init__.py
  • ai_wrapper/llama_client.py
  • ai_wrapper/llm_engine.py
  • ai_wrapper/ollama_wrapper.py
  • config.yaml
  • conftest.py
  • docker-compose.yml
  • docker/neurorift/entrypoint.sh
  • docs/OPENCLAW_INTEGRATION_AUDIT.md
  • docs/WEB_MODE.md
  • examples/notifications.py
  • exploits/__init__.py
  • exploits/exploit_bridge.py
  • exploits/poc_generator.py
  • exploits/shellcode/build_shellcode.sh
  • exploits/shellcode/linux_x64_exec
  • exploits/shellcode/linux_x64_exec.asm
  • exploits/shellcode/linux_x64_shell
  • exploits/shellcode/linux_x64_shell.asm
  • exploits/shellcode/rop_gadgets
  • exploits/shellcode/rop_gadgets.asm
  • fixtures/test_scope.txt
  • flake8_mycode.txt
  • flake8_results.txt
  • flake8_serious.txt
  • install_script.sh
  • integrations/openclaw/openclaw_gateway_adapter.py
  • main.py
  • model_capability_check.py
  • modules/ai/agent.py
  • modules/ai/agent_context.py
  • modules/ai/agents.py
  • modules/ai/ai_assistant.py
  • modules/ai/ai_controller.py
  • modules/ai/ai_integration.py
  • modules/ai/ai_orchestrator.py
  • modules/ai/mode_governor.py
  • modules/ai/orchestrator.py
  • modules/ai/task_memory.py
  • modules/config/config_wizard.py
  • modules/cve_collector/__init__.py
  • modules/cve_collector/cve_collector.py
  • modules/darkweb/__init__.py
  • modules/darkweb/robin/config.py
  • modules/darkweb/robin/llm.py
  • modules/darkweb/robin/llm_utils.py
  • modules/darkweb/robin/runner.py
  • modules/darkweb/robin/scrape.py
  • modules/darkweb/robin/search.py
  • modules/darkweb/robin/ui.py
  • modules/exploit/exploit_module.py
  • modules/exploit_generator/__init__.py
  • modules/exploit_generator/exploit_generator.py
  • modules/exploit_testing/__init__.py
  • modules/exploit_testing/exploit_testing.py
  • modules/orchestration/__init__.py
  • modules/orchestration/data_models.py
  • modules/orchestration/execution_manager.py
  • modules/recon/__init__.py
  • modules/recon/recon.py
  • modules/recon/recon_module.py
  • modules/reporting/__init__.py
  • modules/reporting/reporting.py
  • modules/scan/scan_module.py
  • modules/session/__init__.py
  • modules/session/autosave_service.py
  • modules/session/session_cli.py
  • modules/session/session_manager.py
  • modules/session/session_serializer.py
  • modules/tool_manager/__init__.py
  • modules/tool_manager/tool_manager.py
  • modules/tools/base.py
  • modules/tools/wrappers/amass.py
  • modules/tools/wrappers/ike_scan.py
  • modules/tools/wrappers/masscan.py
  • modules/tools/wrappers/metasploit.py
  • modules/tools/wrappers/mitmproxy.py
  • modules/tools/wrappers/netcat.py
  • modules/tools/wrappers/nmap.py
  • modules/tools/wrappers/sqlmap.py
  • modules/tools/wrappers/unicornscan.py
  • modules/tools/wrappers/wireshark.py
  • modules/web/bridge_server.py
  • modules/web/dashboard.py
  • modules/web/tunnel_manager.py
  • modules/web/ui/orchestration_view.py
  • modules/web/ui/session_view.py
  • modules/web/web_module.py
  • network/Makefile
  • network/README.md
  • network/network_bridge.py
  • network/packet_crafter.c
  • network/raw_socket.c
  • network/tcp_probe.c
  • neurorift/__init__.py
  • neurorift/channels/__init__.py
  • neurorift/channels/channel_router.py
  • neurorift/channels/cli_channel.py
  • neurorift/channels/discord_channel.py
  • neurorift/channels/telegram_channel.py
  • neurorift/channels/web_channel.py
  • neurorift/clawhub/__init__.py
  • neurorift/clawhub/clawhub_api.py
  • neurorift/clawhub/clawhub_client.py
  • neurorift/clawhub/clawhub_resolver.py
  • neurorift/cli/__init__.py
  • neurorift/cli/neurorift_cli.py
  • neurorift/config/__init__.py
  • neurorift/config/channel_config.py
  • neurorift/config/model_config.py
  • neurorift/config/settings.py
  • neurorift/core/__init__.py
  • neurorift/core/agent_loop.py
  • neurorift/core/agent_manager.py
  • neurorift/core/planner.py
  • neurorift/core/task_router.py
  • neurorift/execution/__init__.py
  • neurorift/execution/command_runner.py
  • neurorift/execution/resource_limits.py
  • neurorift/execution/retry_manager.py
  • neurorift/execution/sandbox_runner.py
  • neurorift/gateway/__init__.py
  • neurorift/gateway/api_server.py
  • neurorift/gateway/auth_manager.py
  • neurorift/gateway/websocket_gateway.py
  • neurorift/memory/__init__.py
  • neurorift/memory/long_term_memory.py
  • neurorift/memory/memory_compaction.py
  • neurorift/memory/short_term_memory.py
  • neurorift/memory/vector_memory.py
  • neurorift/models/__init__.py
  • neurorift/models/context_builder.py
  • neurorift/models/model_failover.py
  • neurorift/models/model_router.py
  • neurorift/models/prompt_builder.py
  • neurorift/sessions/__init__.py
  • neurorift/sessions/session_context.py
  • neurorift/sessions/session_manager.py
  • neurorift/sessions/session_pruner.py
  • neurorift/sessions/session_store.py
  • neurorift/skill_store/__init__.py
  • neurorift/skill_store/cache/__init__.py
  • neurorift/skill_store/installed/__init__.py
  • neurorift/skill_store/installed/recon_scanner/README.md
  • neurorift/skill_store/installed/recon_scanner/__init__.py
  • neurorift/skill_store/installed/recon_scanner/requirements.txt
  • neurorift/skill_store/installed/recon_scanner/skill.json
  • neurorift/skill_store/installed/recon_scanner/skill.py
  • neurorift/skills/__init__.py
  • neurorift/skills/installer.py
  • neurorift/skills/loader.py
  • neurorift/skills/registry.py
  • neurorift/skills/skill_manager.py
  • neurorift/skills/skill_validator.py
  • neurorift/storage/__init__.py
  • neurorift/storage/database.py
  • neurorift/storage/logs_db.py
  • neurorift/storage/memory_db.py
  • neurorift/storage/session_db.py
  • neurorift/tools/__init__.py
  • neurorift/tools/tool_executor.py
  • neurorift/tools/tool_registry.py
  • neurorift/tools/tool_sandbox.py
  • neurorift/tools/tool_validator.py
  • neurorift/utils/__init__.py
  • neurorift/utils/logger.py
  • neurorift/utils/serializer.py
  • neurorift/utils/time_utils.py
  • neurorift/utils/validator.py
  • neurorift_cli.py
  • neurorift_launch.sh
  • neurorift_main.py
  • neurorift_wrapper.sh
  • openclaw.json5
  • prompts/agentic_system.md
  • recon/Cargo.toml
  • recon/recon_bridge.py
  • recon/src/dns_resolver.rs
  • recon/src/endpoint_fuzzer.rs
  • recon/src/http_prober.rs
  • recon/src/main.rs
  • recon/src/port_scanner.rs
  • recon/src/subdomain_enum.rs
  • reporting/__init__.py
  • reporting/reporter.py
  • requirements.txt
  • runtime_environment_check.py
  • scope/__init__.py
  • scope/enforcer.py
  • scope/parser.py
  • screen_control/launcher.py
  • screen_control/python/screen_control.py
  • scripts/analyze_results.py
  • scripts/build_all.sh
  • scripts/diagnose_webmod.sh
  • scripts/download_model.sh
  • scripts/install_deps.sh
  • scripts/openclaw_doctor.py
  • scripts/performance_analysis.py
  • scripts/recon_scan.py
  • scripts/start_llama.sh
  • session/__init__.py
  • session/compressor.py
  • session/state.py
  • setup.py
  • sim
  • system-prompts-and-models-of-ai-tools
  • tests/test_ai_features.py
  • tests/test_notifier.py
  • tests/test_security.py
  • tools/__init__.py
  • tools/auth_bypass.py
  • tools/idor.py
  • tools/open_redirect.py
  • tools/race_condition.py
  • tools/shell_exec.py
  • tools/sqli.py
  • tools/ssrf.py
  • tools/ssti.py
  • tools/xss.py
  • tools/xxe.py
  • uninstall_script.sh
  • utils/__init__.py
  • utils/auth.py
  • utils/cli_utils.py
  • utils/config_manager.py
  • utils/context_builder.py
  • utils/crypto.py
  • utils/logger.py
  • utils/notifier.py
  • utils/report_generator.py
  • utils/security_headers.py
  • utils/security_utils.py
  • utils/session_manager.py
  • utils/stealth_utils.py
  • web-ui/src/app/api/ai/chat/route.ts
  • web-ui/src/components/webmode/panels/SystemHealthPanel.tsx
  • web-ui/src/lib/webmode/adapter/interface.ts
  • web-ui/src/lib/webmode/adapter/prototype.ts
  • web-ui/src/lib/webmode/adapter/real.ts

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/neurocore-integration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

This commit fixes the style issues introduced in 7746ca6 according to the output
from Black.

Details: #41
@codeant-ai codeant-ai Bot added the size:XXL This PR changes 1000+ lines, ignoring generated files label Mar 22, 2026
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Mar 22, 2026

Sequence Diagram

This PR introduces a new NeuroCore based execution path that runs a full autonomous security assessment. The flow now parses scope, enforces scoped tool execution, iterates AI tool calls, and produces a final report.

sequenceDiagram
    participant User
    participant MainPipeline
    participant ScopeSystem
    participant ReconBridge
    participant NeuroCore
    participant ToolRegistry
    participant Reporter

    User->>MainPipeline: Start assessment with scope and target
    MainPipeline->>ScopeSystem: Parse scope and enforce allowed targets
    MainPipeline->>ReconBridge: Run initial DNS and HTTP reconnaissance
    ReconBridge-->>MainPipeline: Return recon context
    MainPipeline->>NeuroCore: Request plan and next tool actions
    NeuroCore->>ToolRegistry: Execute scoped tool calls in loop
    ToolRegistry-->>MainPipeline: Return findings and tool outputs
    MainPipeline->>Reporter: Generate final security report
    Reporter-->>User: Return report location and summary
Loading

Generated by CodeAnt AI

Comment on lines +12 to +23
return self.root / f"{sid}.json"

def save(self, session: SessionContext):
self.path(session.session_id).write_text(
json.dumps(session.__dict__, default=str), encoding="utf-8"
)

def load(self, sid: str):
p = self.path(sid)
return (
SessionContext(**json.loads(p.read_text(encoding="utf-8")))
if p.exists()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path Traversal (🔒 Security, 🔴 High) - This code builds a file path using external input "sid" without checking it, risking access outside the allowed folder, which can expose or modify unintended files. View in Corgea ↗

More Details
🎟️Issue Explanation: This code builds a file path using external input "sid" without checking it, risking access outside the allowed folder, which can expose or modify unintended files.

- The code uses "self.root / f"{sid}.json"" to create paths, but "sid" may include "../" to move outside the restricted directory.
- Attackers can supply input like "../../etc/passwd" to read or change files outside the intended folder, leading to data leaks or damage.
- Since the function deals with file paths tied to sensitive data (".json"), unrestricted input can compromise security or system integrity.

🪄Fix Explanation: The fix sanitizes the session ID by rejecting absolute paths, separator-containing names, and traversal tokens, then ensures the resolved file path lies within the intended root directory, effectively preventing path traversal attacks.
The code resolves "self.root" to an absolute path for consistent comparison using "root = self.root.resolve(strict=True)".
It converts the session ID to a "Path" object and rejects invalid IDs that are absolute, contain multiple parts, or equal "." or "..", blocking path traversal attempts.
The candidate file path is constructed with "(root / f"{candidate_name.name}.json")" and resolved without requiring existence.
It verifies the resolved candidate path is within the root directory by confirming "root in candidate.parents", raising an error otherwise.
This combination prevents an attacker from escaping the restricted directory, closing the vulnerability.

💡Important Instructions: Ensure that all session ID inputs at the API boundary are validated before reaching this method to avoid unexpected exceptions.
Suggested change
return self.root / f"{sid}.json"
def save(self, session: SessionContext):
self.path(session.session_id).write_text(
json.dumps(session.__dict__, default=str), encoding="utf-8"
)
def load(self, sid: str):
p = self.path(sid)
return (
SessionContext(**json.loads(p.read_text(encoding="utf-8")))
if p.exists()
# Sanitize session ID to prevent path traversal and enforce containment within the store root
root = self.root.resolve(strict=True)
candidate_name = Path(sid)
# Reject absolute paths and any path containing separators or traversal components
if candidate_name.is_absolute() or len(candidate_name.parts) != 1 or candidate_name.name in (".", ".."):
raise ValueError("invalid session id")
candidate = (root / f"{candidate_name.name}.json").resolve(strict=False)
# Verify the path is within the root directory after resolution
if root not in candidate.parents:
raise ValueError("invalid session id")
return candidate

def list(self):
return self.installer.registry.list()

def run(self, skill_name: str, **kwargs):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

External Control of File Name or Path (🔒 Security, 🔴 High) - The code lets user input control file paths in filesystem actions, risking unauthorized access or file changes. This can lead to serious security issues. View in Corgea ↗

More Details
🎟️Issue Explanation: The code lets user input control file paths in filesystem actions, risking unauthorized access or file changes. This can lead to serious security issues.

- The "skill_name" parameter comes from user input and is concatenated directly to "self.installer.installed", controlling the path.
- An attacker can supply names with "../" to navigate directories and access or modify unintended files.
- This is dangerous as the code likely loads or runs important skill files, risking sensitive data or system control.

🪄Fix Explanation: The fix prevents path traversal by validating the skill name format, ensures the skill is installed, and confirms the resolved path stays within the allowed directory before running it.
- Validates "skill_name" to be a non-empty string matching "[A-Za-z0-9_.-]+", preventing malicious path inputs.
- Checks if "skill_name" is in the installed skills allowlist from "self.installer.registry.list()" to block unauthorized access.
- Resolves the absolute path of the skill and confirms it exists on disk with "target.exists()".
- Verifies the resolved "target" path is inside the "self.installer.installed" directory to prevent directory traversal.
- Only after these checks, passes the safe resolved "target" path to "self.loader.run()" for execution.

💡Important Instructions: Ensure the self.installer.registry.list() remains accurate and up-to-date to avoid false negatives or allowlist bypasses when managing installed skills.
diff --git a/neurorift/skills/skill_manager.py b/neurorift/skills/skill_manager.py
index 602135d..d5aa6f9 100644
--- a/neurorift/skills/skill_manager.py
+++ b/neurorift/skills/skill_manager.py
@@ -1,3 +1,4 @@
+import re
 from pathlib import Path
 from neurorift.clawhub.clawhub_client import ClawHubClient
 from neurorift.skills.installer import SkillInstaller
@@ -22,7 +23,24 @@ class SkillManager:
         return self.installer.registry.list()
 
     def run(self, skill_name: str, **kwargs):
-        return self.loader.run(self.installer.installed / skill_name, **kwargs)
+        # Validate and canonicalize the skill path to prevent path traversal
+        if not isinstance(skill_name, str) or not skill_name:
+            raise ValueError("Invalid skill name")
+        if not re.fullmatch(r"[A-Za-z0-9_.-]+", skill_name):
+            raise ValueError("Invalid skill name")
+        # Allow only installed skills (allowlist)
+        installed_skills = set(self.installer.registry.list())
+        if skill_name not in installed_skills:
+            raise FileNotFoundError(f"Skill not installed: {skill_name}")
+        base = self.installer.installed.resolve()
+        target = base / skill_name
+        if not target.exists():
+            raise FileNotFoundError(f"Skill not found at: {target}")
+        target = target.resolve()
+        # Ensure the resolved target resides within the installed base directory
+        if base != target and base not in target.parents:
+            raise ValueError("Invalid skill path")
+        return self.loader.run(target, **kwargs)
 
     def uninstall(self, skill_name: str):
         return self.installer.uninstall(skill_name)

To apply the fix, Download .patch.


class SkillLoader:
def run(self, skill_dir: Path, **kwargs):
meta = json.loads((skill_dir / "skill.json").read_text(encoding="utf-8"))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Injection (🔒 Security, 🔴 Critical) - The code loads and runs a Python module based on external input without sanitizing it, risking malicious code execution inside the system. View in Corgea ↗

More Details
🎟️Issue Explanation: The code loads and runs a Python module based on external input without sanitizing it, risking malicious code execution inside the system.

- The "entrypoint" in "meta" is from an external file "skill.json", letting attackers specify any file to run.
- Using "exec_module" runs code in the specified file, so if attacker controls "entrypoint", they can execute arbitrary code.
- This is risky because the loaded module’s "run()" function has full access to the program’s environment and sensitive data.

🪄Fix Explanation: The fix restricts module loading to a fixed filename within the skill directory and validates the path to prevent path traversal or symlink bypass, ensuring only trusted code is executed.
"entry = (skill_dir / "skill.py").resolve()" forces a fixed, safe entrypoint filename to eliminate arbitrary file loading risks.
The code verifies "entry" is inside "skill_dir", is a regular file, and not a symlink to block path traversal and symlink attacks.
Safe module name creation uses sanitized directory name via "re.sub(r"[^0-9A-Za-z_]", "_", skill_dir.name)" to prevent malformed or malicious module names.
Module spec and loader presence are checked explicitly, returning errors for invalid specs to avoid undefined behavior.
Loading errors during "spec.loader.exec_module(mod)" are caught to prevent crashes, safely handling malformed modules or runtime errors at import.

💡Important Instructions: Ensure all skill packages follow the fixed entrypoint convention by containing a "skill.py" with a callable run() function; otherwise, skill loading will fail.
diff --git a/neurorift/skills/loader.py b/neurorift/skills/loader.py
index ddb9a53..0c2b65c 100644
--- a/neurorift/skills/loader.py
+++ b/neurorift/skills/loader.py
@@ -1,3 +1,4 @@
+import importlib.util, json, re
 import importlib.util, json
 from pathlib import Path
 
@@ -5,8 +6,28 @@ from pathlib import Path
 class SkillLoader:
     def run(self, skill_dir: Path, **kwargs):
         meta = json.loads((skill_dir / "skill.json").read_text(encoding="utf-8"))
-        entry = skill_dir / meta["entrypoint"]
-        spec = importlib.util.spec_from_file_location(f"skill_{meta['name']}", entry)
+        # Only allow a fixed, safe entrypoint name to avoid path traversal and arbitrary imports
+        entry = (skill_dir / "skill.py").resolve()
+        base_dir = skill_dir.resolve()
+        if not isinstance(meta, dict):
+            return {"error": "invalid metadata"}
+        # Ensure the resolved entrypoint is within the skill directory and is a regular file (no symlinks)
+        try:
+            if entry.is_symlink() or not entry.is_file() or base_dir not in entry.parents:
+                return {"error": "invalid entrypoint"}
+        except OSError:
+            return {"error": "invalid entrypoint"}
+        # Build a safe module name derived from the directory name, not untrusted metadata
+        safe_mod_name = "skill_" + re.sub(r"[^0-9A-Za-z_]", "_", skill_dir.name)
+        spec = importlib.util.spec_from_file_location(safe_mod_name, str(entry))
+        if spec is None or spec.loader is None:
+            return {"error": "invalid module spec"}
         mod = importlib.util.module_from_spec(spec)
-        spec.loader.exec_module(mod)
-        return mod.run(**kwargs) if hasattr(mod, "run") else {"error": "missing run()"}
+        try:
+            spec.loader.exec_module(mod)
+        except Exception:
+            return {"error": "failed to load skill"}
+        run_fn = getattr(mod, "run", None)
+        if not callable(run_fn):
+            return {"error": "missing run()"}
+        return run_fn(**kwargs)

To apply the fix, Download .patch.

@@ -43,12 +44,12 @@ def parse_output(self, raw_output: str) -> Dict[str, Any]:
port_id = port.get("portid")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improper Restriction of XML External Entity Reference (🔒 Security, 🔴 High) - The code processes XML input without restricting external entities, letting attackers include unexpected files via XML entity references, which changes output wrongly. View in Corgea ↗

More Details
🎟️Issue Explanation: The code processes XML input without restricting external entities, letting attackers include unexpected files via XML entity references, which changes output wrongly.

- "ET.fromstring(raw_output)" parses XML but allows external entity references, risking external file inclusion.
- Malicious XML can use entities to load files outside expected data, altering "hosts" list with wrong info.
- This can mislead anyone relying on parsed data (like scanned hosts/ports), affecting security decisions or operations.

🪄Fix Explanation: The fix replaces the standard XML parser with DefusedXML's secure parser and adds handling for DefusedXmlException to prevent XML External Entity (XXE) attacks by blocking dangerous entity resolution during XML parsing.
The code imports "ElementTree" from "defusedxml" instead of the vulnerable standard parser, preventing unsafe entity processing.
Adding "DefusedXmlException" in the except block catches attacks where XML entities try to load external resources.
This update ensures the parser rejects XML with external entities, mitigating unauthorized document embedding risks.

💡Important Instructions: Ensure that all XML input handled by this module uses the updated parsing logic to benefit fully from the security improvements.
diff --git a/modules/tools/wrappers/nmap.py b/modules/tools/wrappers/nmap.py
index 75f2d66..82d47dd 100644
--- a/modules/tools/wrappers/nmap.py
+++ b/modules/tools/wrappers/nmap.py
@@ -1,3 +1,5 @@
+from defusedxml import ElementTree as ET
+from defusedxml.common import DefusedXmlException
 import shutil
 import xml.etree.ElementTree as ET
 from typing import Dict, Any, List
@@ -52,7 +54,7 @@ class NmapTool(BaseTool):
                         )
                 hosts.append({"ip": address, "ports": ports})
             return {"hosts": hosts}
-        except ET.ParseError:
+        except (ET.ParseError, DefusedXmlException):
             return {"error": "Failed to parse Nmap XML", "raw": raw_output[:500]}
 
     def check_installed(self) -> bool:

To apply the fix, Download .patch.

return {"success": True, "name": name, "path": str(dst)}

def uninstall(self, name: str):
dst = self.installed / name
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path Traversal (🔒 Security, 🔴 Critical) - The code lets users delete files by name but doesn't stop them from using special path parts to delete files outside the allowed folder, risking unwanted file removal. View in Corgea ↗

More Details
🎟️Issue Explanation: The code lets users delete files by name but doesn't stop them from using special path parts to delete files outside the allowed folder, risking unwanted file removal.

- "dst = self.installed / name" builds a path from user input "name" without sanitizing it.
- Special elements like "../" in "name" let attackers delete files outside the restricted directory.
- An attacker can pass "../../important_folder" as "name" to remove sensitive files outside "self.installed".

🪄Fix Explanation: The fix prevents directory traversal by validating and resolving the input path before use, ensuring the target directory is always within the allowed installation folder.
- Uses "Path(name).is_absolute()" and checks for forbidden segments ("..", "", ".") to reject absolute paths and path traversal attempts.
- Resolves the base directory with "self.installed.resolve()" to get a canonical path for comparison.
- Combines and resolves the target path "dst = (base_dir / name).resolve()" to prevent symlink or traversal exploits.
- Verifies that "dst" is a subdirectory of "base_dir" using "dst.relative_to(base_dir)" to ensure containment.
- Only removes "dst" if it passes validation and exists, avoiding accidental deletion outside the intended folder.

💡Important Instructions: Ensure self.installed is correctly set and absolute before this method is called to maintain consistent base directory resolution.
Suggested change
dst = self.installed / name
base_dir = self.installed.resolve()
# Reject absolute paths and traversal sequences in 'name'
if Path(name).is_absolute() or any(part in ("..", "", ".") for part in Path(name).parts):
return {"success": False, "error": "invalid name"}
dst = (base_dir / name).resolve()
try:
dst.relative_to(base_dir)
except ValueError:
return {"success": False, "error": "invalid name"}
if dst.exists():
shutil.rmtree(dst)

Comment on lines +21 to +35
meta = json.loads((pkg / "skill.json").read_text(encoding="utf-8"))
name = meta["name"]
dst = self.installed / name
if dst.exists():
shutil.rmtree(dst)
shutil.copytree(pkg, dst)
self.registry.add(name)
return {"success": True, "name": name, "path": str(dst)}

def uninstall(self, name: str):
dst = self.installed / name
if dst.exists():
shutil.rmtree(dst)
self.registry.remove(name)
return {"success": True, "name": name}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path Traversal (🔒 Security, 🔴 Critical) - The code uses external input to build a directory path but doesn't properly check if the path is inside a safe folder, risking unauthorized access or overwriting files outside it. View in Corgea ↗

More Details
🎟️Issue Explanation: The code uses external input to build a directory path but doesn't properly check if the path is inside a safe folder, risking unauthorized access or overwriting files outside it.

- The code sets "dst = self.installed / name", where "name" comes from external JSON data without validation.
- If "name" contains path tricks like "../", it can escape the "self.installed" folder and overwrite important files.
- Example: "name = "../../etc"" leads "shutil.rmtree(dst)" to delete system files outside the intended safe directory.

🪄Fix Explanation: The fix prevents path traversal by validating skill names against a strict pattern and ensuring resolved paths are within the intended installed directory, stopping attackers from escaping to unauthorized locations.
- Added regex validation "re.fullmatch(r"[A-Za-z0-9._-]+", name)" to allow only safe characters in skill names, preventing malicious input.
- Used "Path.resolve()" on both the installed directory and target path to get absolute paths, preventing traversal via symbolic links or ../ segments.
- Verified that the resolved target path is a subpath of the resolved installed directory using "dst.relative_to(installed_root)" to detect and reject traversal attempts.
- Applied the same validation both when installing and uninstalling to ensure consistent protections on all file operations regarding skill names.
- On validation failure, the code returns clear error messages like ""invalid skill name"" or ""path traversal detected"", safely aborting unsafe operations.

💡Important Instructions: Ensure that any other components calling these install/uninstall methods also handle the error responses correctly and avoid proceeding on failure. No further code changes are required.
diff --git a/neurorift/skills/installer.py b/neurorift/skills/installer.py
index 5899d99..44a5ad3 100644
--- a/neurorift/skills/installer.py
+++ b/neurorift/skills/installer.py
@@ -1,3 +1,4 @@
+import json, shutil, re
 import json, shutil
 from pathlib import Path
 from neurorift.skills.skill_validator import SkillValidator
@@ -19,8 +20,15 @@ class SkillInstaller:
         if not ok:
             return {"success": False, "error": f"missing:{missing}"}
         meta = json.loads((pkg / "skill.json").read_text(encoding="utf-8"))
-        name = meta["name"]
-        dst = self.installed / name
+        name = str(meta.get("name", "")).strip()
+        if not name or not re.fullmatch(r"[A-Za-z0-9._-]+", name):
+            return {"success": False, "error": "invalid skill name"}
+        installed_root = self.installed.resolve()
+        dst = (installed_root / name).resolve()
+        try:
+            dst.relative_to(installed_root)
+        except Exception:
+            return {"success": False, "error": "path traversal detected"}
         if dst.exists():
             shutil.rmtree(dst)
         shutil.copytree(pkg, dst)
@@ -28,7 +36,14 @@ class SkillInstaller:
         return {"success": True, "name": name, "path": str(dst)}
 
     def uninstall(self, name: str):
-        dst = self.installed / name
+        if not name or not isinstance(name, str) or not re.fullmatch(r"[A-Za-z0-9._-]+", name):
+            return {"success": False, "error": "invalid skill name"}
+        installed_root = self.installed.resolve()
+        dst = (installed_root / name).resolve()
+        try:
+            dst.relative_to(installed_root)
+        except Exception:
+            return {"success": False, "error": "path traversal detected"}
         if dst.exists():
             shutil.rmtree(dst)
         self.registry.remove(name)

To apply the fix, Download .patch.

Comment thread tools/ssti.py
Comment on lines +45 to +46
target, params={param: payload}, timeout=10, verify=False
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improper Certificate Validation (🔒 Security, 🔴 High) - The code disables SSL certificate verification, making it vulnerable to man-in-the-middle attacks where an attacker can intercept data. View in Corgea ↗

More Details
🎟️Issue Explanation: The code disables SSL certificate verification, making it vulnerable to man-in-the-middle attacks where an attacker can intercept data.

- The code uses "verify=False" in "requests.get()", which skips checking if the server's SSL certificate is valid.
- Without validation, attackers can present fake certificates and intercept or alter sensitive data sent over HTTPS.
- Since the code sends requests to "target" with sensitive params, data like API calls or user info can be compromised.

🪄Fix Explanation: The fix removes "verify=False" from the "requests.get" call, enabling certificate validation and preventing insecure SSL/TLS connections.
"verify=False" was removed to ensure SSL certificates are properly validated during HTTPS requests.
This change enforces trust chain verification, protecting against MITM attacks.
The rest of the call remains unchanged, preserving existing functionality while improving security.

💡Important Instructions: Ensure the target server's SSL certificate is valid and trusted by the system where this code runs to prevent unexpected request failures.
Suggested change
target, params={param: payload}, timeout=10, verify=False
)
target, params={param: payload}, timeout=10

elif cmd_type == "browser_action":
result = await handle_browser_action(command)
else:
raise HTTPException(status_code=400, detail=f"Unknown command type: {cmd_type}")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improper Authentication (🔒 Security, 🔴 Critical) - The code runs commands claimed by users without verifying their identity, risking unauthorized actions. This can let attackers execute commands they're not allowed to. View in Corgea ↗

More Details
🎟️Issue Explanation: The code runs commands claimed by users without verifying their identity, risking unauthorized actions. This can let attackers execute commands they're not allowed to.

- The endpoint "@app.post("/execute")" accepts commands without confirming who sent them or their rights.
- An attacker can send a command with ""type": "tool_execute"" to run restricted tools without proof of permission.
- Lack of authentication checks means anyone can claim any identity and force the system to act on their behalf, risking sensitive operations.

🪄Fix Explanation: The fix enforces strong authentication by validating a provided API token against a securely stored server secret using a timing-attack resistant comparison, preventing unauthorized command execution.
- Imports "hmac" to perform a secure, constant-time comparison of tokens, mitigating timing attacks.
- Retrieves the expected token from environment variables "BRIDGE_API_KEY" or "EXECUTE_API_KEY".
- Extracts the token from possible keys: "api_key", "auth_token", or "token" in the incoming "command" dict.
- Validates presence of expected token, returning HTTP 503 if missing to indicate misconfiguration.
- Compares tokens with "hmac.compare_digest", raising HTTP 401 Unauthorized if tokens don't match.
- Safely removes authentication tokens from the "command" dict after successful validation to avoid misuse downstream.

💡Important Instructions: Ensure environment variables BRIDGE_API_KEY or EXECUTE_API_KEY are securely configured on all deployment environments before enabling this fix.
diff --git a/modules/web/bridge_server.py b/modules/web/bridge_server.py
index 03578bd..0cdcef6 100644
--- a/modules/web/bridge_server.py
+++ b/modules/web/bridge_server.py
@@ -3,6 +3,7 @@ NeuroRift Python Adapter
 Thin bridge between Rust core and Python tools/AI
 """
 
+import hmac
 import os
 from fastapi import FastAPI, HTTPException
 from pydantic import BaseModel
@@ -65,6 +66,18 @@ async def execute_command(command: Dict[str, Any]) -> Response:
     - browser_action: Browser automation action
     """
     try:
+        # Authentication: validate API token from request payload against server secret
+        expected_token = os.environ.get("BRIDGE_API_KEY") or os.environ.get("EXECUTE_API_KEY")
+        auth_token = command.get("api_key") or command.get("auth_token") or command.get("token")
+        if not expected_token:
+            logger.error("Authentication token not configured for /execute; refusing request")
+            raise HTTPException(status_code=503, detail="Service temporarily unavailable")
+        if not auth_token or not hmac.compare_digest(str(auth_token), str(expected_token)):
+            raise HTTPException(status_code=401, detail="Unauthorized")
+        # Remove auth material before further processing
+        command.pop("api_key", None)
+        command.pop("auth_token", None)
+        command.pop("token", None)
         cmd_type = command.get("type")
 
         if cmd_type == "ai_generate":

To apply the fix, Download .patch.

@deepsource-io
Copy link
Copy Markdown
Contributor

deepsource-io Bot commented Mar 22, 2026

DeepSource Code Review

We reviewed changes in 334b4f0...d1961ec on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
Python Mar 22, 2026 4:38p.m. Review ↗

Comment thread modules/web/web_module.py
Comment on lines 136 to 156
cmd = [
"ffuf",
"-u", url,
"-w", self.wordlist,
"-mc", "200,204,301,302,307,401,403",
"-o", temp_output,
"-of", "json",
"-s" # silent
"-u",
url,
"-w",
self.wordlist,
"-mc",
"200,204,301,302,307,401,403",
"-o",
temp_output,
"-of",
"json",
"-s", # silent
]

try:
await self._run_command(" ".join(cmd))

if os.path.exists(temp_output):
with open(temp_output, 'r') as f:
with open(temp_output, "r") as f:
data = json.load(f)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OS Command Injection (🔒 Security, 🔴 Critical) - The code builds a command string using external input "url" without cleaning it, which can let attackers run harmful OS commands. View in Corgea ↗

More Details
🎟️Issue Explanation: The code builds a command string using external input "url" without cleaning it, which can let attackers run harmful OS commands.

- The "cmd" list uses "url" directly, so if it has special OS characters, it changes the command behavior.
- Joining "cmd" with spaces and running it lets an attacker inject commands, like adding “; rm -rf /” in "url".
- Since the code runs "ffuf" (a tool for web fuzzing) with this input, an attacker can exploit it to execute dangerous OS commands.

🪄Fix Explanation: The fix prevents command injection by validating the input for dangerous shell metacharacters before use and switches from shell-based command execution to a safe API that avoids shell invocation.
- Input validation added using "re.search(r'[\s;&|`$<>\\()]', target)" to block suspicious characters that can alter shell commands.
- If invalid characters are found, the code logs an error and returns early, preventing unsafe command execution.
- Replaced the vulnerable "await self._run_command(" ".join(cmd))", which executed commands via the shell, with "asyncio.create_subprocess_exec(*cmd)" to bypass the shell entirely.
- Captured stdout and stderr asynchronously, allowing error detection without exposing shell interpretation risks.
- Maintain original command structure but execute it as a safe subprocess, fully mitigating injection possibilities.

💡Important Instructions: Ensure that the upstream code provides URLs properly encoded and does not allow encoded injection payloads to bypass re.search filtering.
diff --git a/modules/web/web_module.py b/modules/web/web_module.py
index b1e018b..24fea85 100644
--- a/modules/web/web_module.py
+++ b/modules/web/web_module.py
@@ -121,6 +121,10 @@ class WebModule:
             return []
 
         # Ensure target has protocol and trailing slash for FUZZ
+        # Validate target to block shell metacharacters and whitespace
+        if re.search(r'[\s;&|`$<>\\()]', target):
+            self.logger.error("Invalid characters in target: potential command injection attempt.")
+            return []
         url = target
         if "://" not in url:
             url = f"http://{url}"
@@ -149,7 +153,19 @@ class WebModule:
         ]
 
         try:
-            await self._run_command(" ".join(cmd))
+            # Execute ffuf safely without invoking a shell
+            process = await asyncio.create_subprocess_exec(
+                *cmd,
+                stdout=asyncio.subprocess.PIPE,
+                stderr=asyncio.subprocess.PIPE,
+            )
+            stdout, stderr = await process.communicate()
+            if process.returncode != 0:
+                self.logger.error(
+                    "ffuf exited with code %s: %s",
+                    process.returncode,
+                    stderr.decode(errors="ignore"),
+                )
 
             if os.path.exists(temp_output):
                 with open(temp_output, "r") as f:

To apply the fix, Download .patch.

@corgea
Copy link
Copy Markdown

corgea Bot commented Mar 22, 2026

🐕 Corgea found the following new SCA issues in the codebase:

Package CVE Severity Version Fixed Version Ecosystem Summary
next CVE-2026-27980 MEDIUM 16.1.6 16.1.7 npm Next.js: Unbounded next/image disk cache growth can exhaust storage

Showing 1 out of 5 findings. See full results

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Mar 22, 2026

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • CLI Async Bug
    The CLI now invokes several async methods from LocalAIClient as if they were synchronous. This will make the connection check always truthy, return coroutine objects for model listing, and pass an unexecuted coroutine into the nmap analysis path. Please validate the entrypoint uses an async wrapper or asyncio.run(...) for these calls.

  • Pipeline Crash
    The AI pipeline branch references args.target, but no such CLI argument is defined here, and the nmap path also accesses orchestrator.analyzer even though AIOrchestrator does not initialize that attribute. Running those options will raise at runtime.

  • Import Failure
    The planner prompt file is read during module import, so importing ai.planner now depends on the current working directory and the file being present at that exact relative path. Please verify this will not fail in CLI, test, or deployment entry points.

  • Tool Arguments
    Validate and catch JSON parsing errors for tool-call arguments. A single malformed arguments payload can currently abort the entire executor loop instead of returning an error for that call and continuing safely.

  • Possible Bug
    The CPE parser accesses parts[5] after only checking for at least 5 segments. Malformed or non-standard CPE strings can still trigger an IndexError here, so this path should be validated more defensively before matching targets.

  • Scope Bypass
    The wildcard scope check appears to allow the apex domain to pass when only a subdomain wildcard was intended. This should be validated carefully because it can approve targets that were meant to stay out of scope.

  • Validation Bug
    The path traversal filter checks the resolved absolute path for substrings such as /etc/, /root/, /proc/, and /sys/. This can reject legitimate paths that merely contain those names in a normal project directory. Please verify the rule only blocks true escapes from the allowed base directory.

  • Parsing Bug
    The bare-host normalization logic may corrupt IPv6 literals by stripping on the first port-like pattern. This can cause valid in-scope IPv6 targets to be rejected or misclassified during scope checks.

  • Runtime Bug
    The subprocess.run invocation appears to pass the command twice, which should be reviewed because it will prevent the tool from executing any allowed binary at runtime. Verify that the command is passed only once as the argument list.

  • Parsing Bug
    The command parsing logic uses a naive whitespace split, so quoted arguments and paths with spaces will be broken into incorrect tokens. This can make valid tool invocations fail or behave differently than intended.

  • Packet Format
    The IP header is assembled with host-order values for fields that normally need network-byte order, and the checksum is computed before verifying the header layout matches what would be transmitted. Please validate that the crafted bytes are actually correct for downstream use.

  • Zero Concurrency
    A concurrency value of 0 creates a semaphore that never allows progress, causing every spawned scan task to wait indefinitely. The input should be clamped or rejected before constructing the semaphore.

  • Possible Bug
    The IPv4 parsing result is ignored, so invalid --target values can leave addr.sin_addr at its zeroed default and produce a misleading probe result instead of rejecting the input.

  • Runtime Error
    The socket error check does not verify the getsockopt return value before reading so_error, which can lead to an uninitialized error code being used to classify the port state if the socket query fails.

  • Input Validation
    CLI values are parsed with permissive conversions and no range or validity checks, so invalid IPs, out-of-range ports, or TTL values can silently produce malformed packet fields. Please verify that bad inputs are rejected before building the packet.

Comment thread ai/executor.py Outdated

logger = logging.getLogger(__name__)

EXECUTOR_SYSTEM = open("ai/prompts/executor_system.txt").read()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Loading the system prompt with a raw relative open(...).read() depends on current working directory and may fail when launched from another path. Use a path derived from the module location to make prompt loading reliable. [possible bug]

Severity Level: Major ⚠️
- ❌ Executor import fails outside repository-root working directory.
- ⚠️ CLI/automation startup becomes directory-dependent.
Suggested change
EXECUTOR_SYSTEM = open("ai/prompts/executor_system.txt").read()
EXECUTOR_SYSTEM = (__import__("pathlib").Path(__file__).resolve().parent / "prompts" / "executor_system.txt").read_text(encoding="utf-8")
Steps of Reproduction ✅
1. Run project entry using absolute script path from outside repo root (imports still
happen in `main.py:71` via `from ai.executor import Executor`).

2. During module import, top-level statement in `ai/executor.py:13` executes immediately.

3. `open("ai/prompts/executor_system.txt")` resolves against current working directory,
not module directory.

4. If cwd is not repository root, import fails with `FileNotFoundError`, preventing
execute phase startup.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** ai/executor.py
**Line:** 13:13
**Comment:**
	*Possible Bug: Loading the system prompt with a raw relative `open(...).read()` depends on current working directory and may fail when launched from another path. Use a path derived from the module location to make prompt loading reliable.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Comment thread ai/executor.py
)
neurocore.unload_model()

if not response or response.get("error"):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The response is assumed to be a dictionary and .get() is called unconditionally; if the backend returns a non-dict value, this raises AttributeError and crashes the run. Validate the response type before key access. [type error]

Severity Level: Major ⚠️
- ❌ Executor crashes on non-dict NeuroCore responses.
- ⚠️ Attack-plan execution halts before tool dispatch.
Suggested change
if not response or response.get("error"):
if not isinstance(response, dict) or response.get("error"):
Steps of Reproduction ✅
1. Assessment enters executor inference loop (`main.py:135` -> `ai/executor.py:66-71`).

2. Guard check at `ai/executor.py:73` assumes truthy `response` has `.get(...)`.

3. `Planner.create_plan` already treats non-dict responses as realistic
(`ai/planner.py:74-77`), showing NeuroCore outputs are not strictly dict-only.

4. When executor receives a truthy non-dict response, `.get` access at line 73 raises
`AttributeError`, terminating execution.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** ai/executor.py
**Line:** 73:73
**Comment:**
	*Type Error: The response is assumed to be a dictionary and `.get()` is called unconditionally; if the backend returns a non-dict value, this raises `AttributeError` and crashes the run. Validate the response type before key access.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Comment thread ai/executor.py
Comment on lines +101 to +103
fn_args = json.loads(
call.get("function", {}).get("arguments", "{}")
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Tool-call arguments are parsed as JSON without error handling, so malformed model output raises JSONDecodeError and aborts execution. Catch decode failures and continue safely. [possible bug]

Severity Level: Critical 🚨
- ❌ Executor loop aborts on malformed tool argument JSON.
- ⚠️ Remaining planned tool steps never execute.
Suggested change
fn_args = json.loads(
call.get("function", {}).get("arguments", "{}")
)
raw_args = call.get("function", {}).get("arguments", "{}")
try:
fn_args = json.loads(raw_args)
except json.JSONDecodeError:
fn_args = {}
Steps of Reproduction ✅
1. `Executor.run` handles model tool calls at `ai/executor.py:90-99` after `main.py:135`
starts execution phase.

2. Tool-call payloads are passed through from model output (`response["calls"]`) without
validation (`ai/executor.py:95-103`).

3. If `call.function.arguments` is malformed JSON, `json.loads(...)` at
`ai/executor.py:101-103` raises `JSONDecodeError`.

4. Exception is unhandled in loop, so execution stops before `_dispatch`
(`ai/executor.py:106`) and remaining plan steps are skipped.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** ai/executor.py
**Line:** 101:103
**Comment:**
	*Possible Bug: Tool-call arguments are parsed as JSON without error handling, so malformed model output raises `JSONDecodeError` and aborts execution. Catch decode failures and continue safely.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Comment thread ai/executor.py
if not fn:
return {"error": f"Unknown tool: {tool_name}"}
try:
result = fn(**args)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Calling tools directly inside the async executor can break tools that start their own event loop and also blocks the running loop; this makes some tools fail at runtime and stalls other async work. Run synchronous tools in a worker thread and only await true async tools. [possible bug]

Severity Level: Critical 🚨
- ❌ Race-condition tool execution fails in executor async path.
- ⚠️ Long synchronous tools block executor event loop.
Suggested change
result = fn(**args)
import asyncio
if asyncio.iscoroutinefunction(fn):
result = await fn(**args)
else:
result = await asyncio.to_thread(fn, **args)
Steps of Reproduction ✅
1. Run orchestrated assessment via `main.py:179` (`asyncio.run(run_assessment(...))`),
which awaits `Executor.run` at `main.py:135`.

2. `Executor._dispatch` at `ai/executor.py:130` directly executes `fn(**args)` inside
active event loop context.

3. Tool registry includes `RaceConditionTool.run` from `main.py:55-60`; that method calls
`asyncio.run(...)` at `tools/race_condition.py:47`.

4. When planner selects `race_condition`, nested `asyncio.run` executes under running loop
and fails (`RuntimeError`), producing tool failure instead of race-test execution.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** ai/executor.py
**Line:** 130:130
**Comment:**
	*Possible Bug: Calling tools directly inside the async executor can break tools that start their own event loop and also blocks the running loop; this makes some tools fail at runtime and stalls other async work. Run synchronous tools in a worker thread and only await true async tools.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Comment thread ai/planner.py Outdated
reasoning: str = ""


PLANNER_SYSTEM = open("ai/prompts/planner_system.txt").read()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The prompt file is loaded using a process-relative path at import time, which can fail with FileNotFoundError when the app is started from a different working directory. Resolve the path from the module location so imports are stable regardless of where the process is launched. [possible bug]

Severity Level: Critical 🚨
-`main.py` startup can crash before planning phase.
- ⚠️ Launching from non-repo directories becomes unreliable.
- ⚠️ Import-time failure blocks `run_assessment` initialization path.
Suggested change
PLANNER_SYSTEM = open("ai/prompts/planner_system.txt").read()
PLANNER_SYSTEM = (
__import__("pathlib")
.Path(__file__)
.resolve()
.parent.joinpath("prompts", "planner_system.txt")
.read_text(encoding="utf-8")
)
Steps of Reproduction ✅
1. Start from any directory outside repo root (for example `/tmp`) and run the documented
entry script by absolute path (`python /workspace/NeuroRift/main.py ...`); usage is
documented in `main.py:6` and `main.py:158`.

2. `run_assessment()` in `main.py:65` executes `from ai.planner import Planner` at
`main.py:70`.

3. Importing `ai/planner.py` evaluates module-level code immediately, including
`PLANNER_SYSTEM = open("ai/prompts/planner_system.txt").read()` at `ai/planner.py:23`.

4. Because that path is CWD-relative (not file-relative), Python raises
`FileNotFoundError` when CWD is not `/workspace/NeuroRift`, and assessment startup aborts
before planning begins.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** ai/planner.py
**Line:** 23:23
**Comment:**
	*Possible Bug: The prompt file is loaded using a process-relative path at import time, which can fail with `FileNotFoundError` when the app is started from a different working directory. Resolve the path from the module location so imports are stable regardless of where the process is launched.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Comment thread modules/ai/agents.py
Comment on lines 69 to +72
for step in plan_data:
# Ensure target is present, if not use task mentions or safe default (should be handled by AI)
# Safely parse the step
tool_name = step.get("tool_name")
if not tool_name:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Planned steps are accepted even when tool_name is not in available_tools, which can later trigger a hard failure in ExecutionManager (Tool ... not found). Filter hallucinated/invalid tool names during plan parsing to keep the execution pipeline stable. [logic error]

Severity Level: Critical 🚨
- ❌ Hallucinated tool steps break orchestration execution stage.
- ⚠️ Planner output reliability drops despite valid tool list.
Suggested change
for step in plan_data:
# Ensure target is present, if not use task mentions or safe default (should be handled by AI)
# Safely parse the step
tool_name = step.get("tool_name")
if not tool_name:
valid_tool_names = {t["name"] for t in available_tools}
for step in plan_data:
# Safely parse the step
tool_name = step.get("tool_name")
if not tool_name or tool_name not in valid_tool_names:
Steps of Reproduction ✅
1. Generate a plan through normal entrypoints: CLI `vf.planner.create_plan(...)`
(`neurorift_main.py:1250-1251`) or UI `planner.create_plan(...)`
(`modules/web/ui/orchestration_view.py:108-110`).

2. `NRPlanner.create_plan` currently accepts any `tool_name` returned by LLM
(`modules/ai/agents.py:68-79`) without checking against provided `available_tools`.

3. During execution, `ExecutionManager.execute_tool` resolves tool by name and raises
`ValueError` for unknown names (`modules/orchestration/execution_manager.py:53-55`).

4. This invalid-plan step causes execution failure/crash path in orchestrated pipeline,
instead of being filtered during planning.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** modules/ai/agents.py
**Line:** 69:72
**Comment:**
	*Logic Error: Planned steps are accepted even when `tool_name` is not in `available_tools`, which can later trigger a hard failure in `ExecutionManager` (`Tool ... not found`). Filter hallucinated/invalid tool names during plan parsing to keep the execution pipeline stable.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Comment thread modules/ai/agents.py

# Simulated Execution Wait State (Manus-style)
# The agent blocks here and receives environment feedback upon completion.
result = await self.manager.execute_tool(req, context)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: execute_plan does not handle exceptions from execute_tool, but execute_tool can raise before its internal try/except (for unknown tools, permission checks, or input validation). This will abort the whole orchestration flow unexpectedly instead of failing the current step gracefully. [possible bug]

Severity Level: Critical 🚨
- ❌ Orchestrated run aborts before analysis/report generation.
- ⚠️ Partial tool results lost from operator loop.
Suggested change
result = await self.manager.execute_tool(req, context)
try:
result = await self.manager.execute_tool(req, context)
except Exception as e:
print(f"[OPERATOR] Step failed with exception: {e}")
break
Steps of Reproduction ✅
1. Start orchestrated execution path: `neurorift_main.py:1266-1267` calls
`vf.operator.execute_plan(requests, context)`.

2. In `NROperator.execute_plan` (`modules/ai/agents.py:90-106`), each step directly awaits
`self.manager.execute_tool(...)` with no local try/except.

3. `ExecutionManager.execute_tool` raises exceptions before its internal execution
try-block for several realistic conditions: unknown tool
(`modules/orchestration/execution_manager.py:53-55`), mode violation (`58-69`), invalid
input (`75-76`).

4. Those exceptions bubble out of `execute_plan`, aborting orchestrated flow before
analysis/report stages (`neurorift_main.py:1270-1283`).
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** modules/ai/agents.py
**Line:** 99:99
**Comment:**
	*Possible Bug: `execute_plan` does not handle exceptions from `execute_tool`, but `execute_tool` can raise before its internal try/except (for unknown tools, permission checks, or input validation). This will abort the whole orchestration flow unexpectedly instead of failing the current step gracefully.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Comment thread modules/ai/agents.py

data = json.loads(cleaned)
for item in data:
finding = Finding(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The analysis loop assumes every parsed JSON item is a dictionary and calls .get directly; if one malformed/non-object entry appears, it raises and the broad except drops all findings. Validate item type per entry so one bad item does not discard the full analysis. [type error]

Severity Level: Major ⚠️
- ⚠️ AI findings list truncates after malformed entry.
- ⚠️ Analysts miss later valid findings in same response.
Suggested change
finding = Finding(
if not isinstance(item, dict):
continue
Steps of Reproduction ✅
1. Trigger analysis via CLI (`neurorift_main.py:1270-1271`) or UI button
(`modules/web/ui/orchestration_view.py:130-133`), both calling
`NRAnalyst.analyze_results`.

2. `NRAnalyst.analyze_results` parses LLM JSON and iterates items
(`modules/ai/agents.py:156-165`).

3. If any parsed array element is non-dict, `item.get(...)` raises `AttributeError` inside
the broad try block (`modules/ai/agents.py:149-167`).

4. Function logs parse error and exits loop early, skipping remaining entries; only items
processed before the malformed one survive.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** modules/ai/agents.py
**Line:** 158:158
**Comment:**
	*Type Error: The analysis loop assumes every parsed JSON item is a dictionary and calls `.get` directly; if one malformed/non-object entry appears, it raises and the broad except drops all findings. Validate item type per entry so one bad item does not discard the full analysis.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Comment thread modules/ai/agents.py
4. Remediation Recommendations
"""
return await self.ollama.generate(prompt)
return await self.llm_client.generate(prompt)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: generate_report returns the raw result of llm_client.generate, but that call can return None when AI is disabled or unavailable. Callers treat the result as a string (write to file, markdown rendering, slicing), which will raise runtime TypeError. Normalize the return to a guaranteed string. [null pointer]

Severity Level: Critical 🚨
- ❌ Orchestrated CLI report save crashes when AI unavailable.
- ⚠️ Web report rendering receives null markdown content.
Suggested change
return await self.llm_client.generate(prompt)
report = await self.llm_client.generate(prompt)
return report or "# Report generation failed: AI service unavailable."
Steps of Reproduction ✅
1. Run orchestrated CLI flow in `neurorift_main.py:1246-1256`; it calls
`vf.scribe.generate_report(...)` at `neurorift_main.py:1275`.

2. Make AI unavailable (supported by code): `LocalAIClient.generate()` returns `None` when
`AI_ENABLED` is false (`modules/ai/ai_integration.py:73-74`) or service is down
(`modules/ai/ai_integration.py:76-77, 91`).

3. `NRScribe.generate_report()` at `modules/ai/agents.py:176-196` returns that `None`
directly.

4. CLI then executes string operations on `report`: `f.write(report)` and `report[:1000]`
at `neurorift_main.py:1280-1283`, causing runtime type errors for `None`.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** modules/ai/agents.py
**Line:** 196:196
**Comment:**
	*Null Pointer: `generate_report` returns the raw result of `llm_client.generate`, but that call can return `None` when AI is disabled or unavailable. Callers treat the result as a string (write to file, markdown rendering, slicing), which will raise runtime `TypeError`. Normalize the return to a guaranteed string.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

self.prompt_dir = prompt_dir
self.ollama = OllamaClient()
self.llm_client = LocalAIClient()
self.prompts = self._load_prompts()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The orchestrator now creates only the LLM client but never initializes an analyzer, while CLI code still expects orchestrator.analyzer. This will raise an AttributeError at runtime when nmap analysis is requested. Initialize the analyzer in the constructor using the same LLM client. [possible bug]

Severity Level: Major ⚠️
-`--analyze-nmap` CLI path crashes immediately.
- ⚠️ AI nmap analysis feature becomes unusable.
Suggested change
self.prompts = self._load_prompts()
self.analyzer = AIAnalyzer(self.llm_client)
Steps of Reproduction ✅
1. Run module CLI path `python -m modules.ai.ai_integration --analyze-nmap <file>` to
enter `if __name__ == "__main__"` in `modules/ai/ai_integration.py`.

2. Control reaches `elif args.analyze_nmap:` at `modules/ai/ai_integration.py:565` and
reads file content.

3. Code calls `orchestrator.analyzer.analyze_nmap_output(content)` at
`modules/ai/ai_integration.py:567`.

4. `AIOrchestrator.__init__` only sets `self.llm_client`
(`modules/ai/ai_integration.py:400` in PR hunk) and never defines `self.analyzer`, causing
`AttributeError`.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** modules/ai/ai_integration.py
**Line:** 401:401
**Comment:**
	*Possible Bug: The orchestrator now creates only the LLM client but never initializes an analyzer, while CLI code still expects `orchestrator.analyzer`. This will raise an `AttributeError` at runtime when nmap analysis is requested. Initialize the analyzer in the constructor using the same LLM client.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Mar 22, 2026

CodeAnt AI finished reviewing your PR.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7746ca65e9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docker-compose.yml Outdated
OPENCLAW_GATEWAY_ADDR: "0.0.0.0:18789"
NEURORIFT_BRIDGE_URL: "http://neurorift-core:8766"
OLLAMA_HOST: "http://ollama:11434"
OLLAMA_HOST: "http://host.docker.internal:8080"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Pass LLAMA_HOST to gateway container

I checked docker-compose.yml against integrations/openclaw/openclaw_gateway_adapter.py: the adapter now requires LLAMA_HOST in REQUIRED_ENV, but this service still exports OLLAMA_HOST. In this commit that makes normalize_env() fail fast with Missing required environment variable: LLAMA_HOST, so the gateway process cannot start under compose until the env key is renamed (or both keys are provided).

Useful? React with 👍 / 👎.

Comment thread docker/neurorift/entrypoint.sh Outdated
for i in $(seq 1 $MAX_RETRIES); do
if curl -sf "${OLLAMA_HOST}/api/tags" > /dev/null 2>&1; then
echo "✅ Ollama is ready (attempt ${i}/${MAX_RETRIES})"
if curl -sf "${LLAMA_HOST}/api/tags" > /dev/null 2>&1; then
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Probe llama.cpp with a valid readiness endpoint

The entrypoint was switched to llama.cpp, but the health probe still hits ${LLAMA_HOST}/api/tags, which is an Ollama-specific route. With the server launched by scripts/start_llama.sh (python -m llama_cpp.server), this check never returns healthy, so startup always waits through the full retry loop and does not actually verify LLM readiness before launching the bridge.

Useful? React with 👍 / 👎.

Comment thread ai/planner.py Outdated
reasoning: str = ""


PLANNER_SYSTEM = open("ai/prompts/planner_system.txt").read()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Resolve planner prompt path from module directory

This prompt file is opened using a process-relative path at import time. If NeuroRift is started from any working directory other than the repo root (for example via installed CLI wrappers), importing ai.planner raises FileNotFoundError before planning runs. Building the path from __file__ avoids this runtime break.

Useful? React with 👍 / 👎.

Arunking9 and others added 5 commits March 22, 2026 16:00
This commit fixes the style issues introduced in 42ea9c6 according to the output
from Black.

Details: #41
This commit fixes the style issues introduced in ac1a8d8 according to the output
from Black.

Details: #41
@Neuro-Rift Neuro-Rift merged commit 49ae2cb into main Mar 22, 2026
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL This PR changes 1000+ lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant