Skip to content

Add Codex-powered TraceTree workbench - #53

Draft
tejasprasad2008-afk wants to merge 12 commits into
mainfrom
codex/dashboard-hackathon
Draft

Add Codex-powered TraceTree workbench#53
tejasprasad2008-afk wants to merge 12 commits into
mainfrom
codex/dashboard-hackathon

Conversation

@tejasprasad2008-afk

@tejasprasad2008-afk tejasprasad2008-afk commented Jul 16, 2026

Copy link
Copy Markdown
Owner

What changed

Adds the TraceTree Workbench dashboard and its supporting API, orchestration, analyzer, model, YARA, sandbox, and test wiring.

The dashboard provides:

  • Dashboard, live-monitoring, history, evidence, signatures, and settings views
  • Command-console support for local TraceTree workflows
  • Search/filter primitives and reusable workbench components
  • Static-disassembly support with bounded file and instruction limits

Why

This branch records the dashboard feature developed during the Codex Hackathon as a single reviewable timeline, without merging it into main during the event.

Validation

  • npm run build in frontend/
  • Targeted Python tests were not run because the repository virtualenv does not include pytest.
  • Full scan workflows require a reachable Docker daemon.

Local testing

git clone --branch codex/dashboard-hackathon https://github.com/tejasprasad2008-afk/TraceTree.git
cd TraceTree
cascade-analyze dashboard

For controlled malware-research samples, use an authorized isolated environment and follow the access requirements at MalwareBazaar. Do not download or execute unknown samples on a personal host.

Scope exclusions

This PR intentionally excludes the incomplete macOS app, local runtime state, generated artifacts, virtual environments, sample/model binaries, and experimental scripts.

Summary by CodeRabbit

  • New Features
    • Launched TraceTree Workbench with dashboard, live monitor, scan history, evidence, signatures, and settings, plus a command console.
    • Added CVE syncing/enrichment and static disassembly viewing for quarantine hits.
  • Bug Fixes
    • Hardened local API behavior (loopback-only when keys aren’t set) and improved streaming output.
    • Improved scoring and detection reliability with updated signatures/rules.
  • Documentation
    • Updated setup and prerequisites for first-run Workbench setup.
  • Style
    • Refreshed Workbench theming and table/terminal UI interactions.

tejasprasad2008-afk and others added 2 commits July 17, 2026 00:44
…ndering as caution

build_behavior_receipt only ever emitted "suspicious"/"clean" — "malicious"
was never surfaced, so the API's decision->level map (malicious->danger)
was dead code and high-confidence detections downgraded to yellow "caution"
in the UI. Now emits "malicious" when risk_score >= 75. Mirrored the same
lowercase clean/suspicious/malicious contract in the TS orchestrator's
SQLite scan persistence, which previously wrote MALICIOUS/CLEAN.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Repository owner deleted a comment from coderabbitai Bot Jul 19, 2026
@tejasprasad2008-afk
tejasprasad2008-afk marked this pull request as ready for review July 19, 2026 08:39
@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The pull request expands TraceTree with a Workbench frontend, broader FastAPI and CLI workflows, standardized analysis adapters, CVE synchronization, quarantine-backed evidence handling, updated detection semantics, and new model retraining and disassembly capabilities.

Changes

Analysis platform

Layer / File(s) Summary
Analyzer framework and detection adapters
monitor/analyzers/*, tests/unit/test_phase_a_wrappers.py
Adds standardized analyzer findings and adapters for YARA, syscall, Random Forest, static disassembly, and AI triage, with parity tests.
Detection scoring and signatures
monitor/parser.py, monitor/signatures.py, monitor/utils.py, monitor/yara.py, ml/detector.py, data/mcp_rce_signatures.yara
Adjusts syscall severity, benign-path handling, malicious confidence, signature matching, YARA rules, and matched-offset output.
Sandbox quarantine and session evidence
sandbox/sandbox.py, cli.py, .gitignore
Adds MCP sandbox handling, malware quarantine extraction, stale quarantine cleanup, YARA sidecar consumption, and persisted YARA findings.
Analysis and orchestration API
api/main.py, retrain_model.py, setup.py, .env.example, README.md
Adds local API-key mode, PTY-based SSE command streaming, scan/history/disassembly/signature/model/Docker endpoints, file scanning, graph persistence, and retraining support.

CVE operations

Layer / File(s) Summary
CVE ingestion and matching
monitor/cve_sync.py
Adds CISA, NVD, and OSV ingestion, relevance filtering, optional Ollama enrichment, audit persistence, target matching, and background synchronization.
CVE CLI integration
cli.py, pyproject.toml, setup.py
Displays matched CVE intelligence during analysis and registers the cascade-cve-sync command.

Workbench UI

Layer / File(s) Summary
Workbench theme and primitives
frontend/app/globals.css, frontend/tailwind.config.ts, frontend/components/workbench/*, docs/CODEX_UI_PROMPT.md
Defines Workbench styling tokens and reusable table, panel, severity, split-pane, filtering, raw-output, and search components.
Workbench shell and screens
frontend/app/page.tsx, frontend/components/workbench/screens/*, frontend/components/workbench/UserMenu.tsx, frontend/components/workbench/FirstRunSetup.tsx
Replaces the prior dashboard with setup-gated tabbed navigation, session-aware history, live monitoring, evidence, signatures, settings, dashboard, and service status screens.
Command console integration
frontend/components/workbench/CommandConsole.tsx, frontend/components/MSDosPrompt.tsx
Adds command presets, streamed execution, cancellation, file selection, terminal scrolling, and CVE enrichment.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 32.95% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding the Codex-powered TraceTree workbench and its supporting features.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/dashboard-hackathon

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.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add TraceTree Workbench dashboard with supporting API, analyzers, and CVE/YARA tooling

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Add Workbench UI with dashboard, live monitor, history, evidence, signatures, and settings
 screens.
• Extend FastAPI gateway to stream CLI output, serve scan history, manage YARA rules, and run
 disassembly.
• Introduce analyzer wrapper interfaces plus CVE sync and parser tweaks to reduce benign false
 positives.
Diagram

graph TD
FE["Workbench UI"] --> API["FastAPI API"] --> CLI["cli.py"] --> SB["Docker sandbox"] --> AN["Analyzers"] --> ST[(".tracetree state")]
FE --> WS["Orchestrator WS"]
FE --> EXT{{"OSV/NVD/CISA"}}
AN --> EXT
subgraph Legend
  direction LR
  _svc["Service/UI"] ~~~ _db[("Local storage")] ~~~ _ext{{"External"}}
end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Replace /api/execute subprocess with a job queue + log tailing
  • ➕ Avoids in-memory state (mock_db) and BackgroundTasks limitations
  • ➕ Better concurrency/cancellation and crash recovery
  • ➕ Naturally supports multi-user / remote deployments
  • ➖ Adds operational complexity (Redis/Celery/etc.)
  • ➖ Overkill for the local-first hackathon scope
2. Model scan history as SQLite (instead of .tracetree/history dirs)
  • ➕ Simpler querying/filtering and pagination for UI
  • ➕ Atomic updates; easier future migrations
  • ➕ Avoids directory parsing edge cases
  • ➖ Requires schema design and migrations
  • ➖ Less transparent/inspectable than plain JSON artifacts
3. Use WebSockets end-to-end for command streaming (instead of SSE + PTY)
  • ➕ Bidirectional control (interactive TTY, resize, cancel)
  • ➕ Single transport for live monitor + console
  • ➖ More complex client/server state handling
  • ➖ PTY lifecycle and security hardening still required

Recommendation: Given the stated goal (hackathon-built, local workbench without merging mid-event), the PR’s approach is reasonable: FastAPI provides a thin local gateway, and PTY-backed SSE makes Rich/Typer output usable in-browser. If this is expected to evolve into a long-lived product feature, prioritize replacing in-memory state + BackgroundTasks with a durable job model, and consider consolidating filesystem-based history into a queryable store (SQLite) before adding more UI filters and views.

Files changed (45) +5672 / -872

Enhancement (32) +4857 / -689
main.pyAdd Workbench-facing API endpoints and PTY-streamed CLI execution +489/-38

Add Workbench-facing API endpoints and PTY-streamed CLI execution

• Extends the FastAPI gateway with local-dev auth behavior (dev-key) and additional CORS origins. Adds /api/execute streaming backed by a PTY to preserve Rich progress output, plus endpoints for session info, scan history, file scan submission, docker status, model info/retrain, signatures read/write, and bounded static disassembly with path restrictions.

api/main.py

cli.pyWire CVE sync, quarantine hygiene, and richer scan output paths +294/-41

Wire CVE sync, quarantine hygiene, and richer scan output paths

• Adds automatic stale quarantine cleanup at analysis start, and wires background CVE sync plus CLI-side CVE match rendering into analysis output. Updates YARA scanning to include quarantined extracted binaries via a .qdir sidecar, and ensures error paths flush output for reliable streaming.

cli.py

mcp_rce_signatures.yaraUpdate MCP-focused YARA signatures +6/-6

Update MCP-focused YARA signatures

• Adjusts the repository’s YARA ruleset used for MCP/server RCE detection and UI editing. Keeps the signature pack aligned with the new signatures editor and analyzer offset reporting.

data/mcp_rce_signatures.yara

layout.tsxMinor layout tweaks for Workbench shell +3/-3

Minor layout tweaks for Workbench shell

• Adjusts the Next.js app layout to support the new Workbench shell styling and structure. Keeps global layout consistent with the updated CSS token model.

frontend/app/layout.tsx

page.tsxReplace legacy dashboard with Workbench shell navigation +365/-582

Replace legacy dashboard with Workbench shell navigation

• Replaces the prior single-page dashboard UI with a Workbench shell that manages navigation across multiple screens and a command console. Normalizes scan row shapes and introduces shared app stats/session state handling.

frontend/app/page.tsx

MSDosPrompt.tsxTweak prompt component for new console/workbench usage +7/-5

Tweak prompt component for new console/workbench usage

• Small adjustments to the MSDosPrompt component to better integrate with the updated UI patterns. Helps maintain compatibility with the Workbench command workflow.

frontend/components/MSDosPrompt.tsx

CommandConsole.tsxAdd streaming command console with CVE extraction/enrichment +452/-0

Add streaming command console with CVE extraction/enrichment

• Implements a Workbench command console that runs local CLI commands via /api/execute and streams output incrementally. Extracts CVE IDs from output and optionally enriches via OSV, with presets for common workflows and basic output hygiene.

frontend/components/workbench/CommandConsole.tsx

DataTable.tsxAdd reusable dense DataTable component +193/-0

Add reusable dense DataTable component

• Introduces a Workbench table component with column definitions, row selection, and severity-aware rendering hooks. Used across Dashboard, Scan History, Evidence, and other screens for consistent dense presentation.

frontend/components/workbench/DataTable.tsx

FilterBar.tsxAdd filter pill bar for active filters +74/-0

Add filter pill bar for active filters

• Adds a compact filter bar component to show and clear active filters in list views. Designed to pair with the HTTPQL-like search query language in scan history.

frontend/components/workbench/FilterBar.tsx

Panel.tsxAdd flat bordered panel container +58/-0

Add flat bordered panel container

• Creates a consistent panel wrapper for Workbench regions (tables, stats, details). Enforces the “no cards inside cards” and flat-border UI constraints from the design spec.

frontend/components/workbench/Panel.tsx

RawWell.tsxAdd scrollable monospace log/output well +59/-0

Add scrollable monospace log/output well

• Adds a raw text well for streaming logs, receipts, and live WebSocket event output. Supports auto-scroll behavior for live monitoring and command output views.

frontend/components/workbench/RawWell.tsx

SearchBar.tsxAdd HTTPQL-style search bar with completions +179/-0

Add HTTPQL-style search bar with completions

• Implements a dense search bar intended for structured queries like verdict/type/file/sha256 filtering. Supports completions and keyboard-friendly workflows for scan history search.

frontend/components/workbench/SearchBar.tsx

SeverityTag.tsxAdd severity tag component and CVSS mapping helpers +40/-0

Add severity tag component and CVSS mapping helpers

• Introduces rectangular severity tags used across Workbench tables and detail views. Includes helper logic for mapping scores (e.g., CVSS) to display severity levels.

frontend/components/workbench/SeverityTag.tsx

SplitPane.tsxAdd persistent split-pane layout for list/detail views +96/-0

Add persistent split-pane layout for list/detail views

• Implements a resizable split-pane with persisted sizing for Workbench screens. Used for Scan History and Evidence detail views to keep dense, IDE-like ergonomics.

frontend/components/workbench/SplitPane.tsx

UserMenu.tsxAdd top-bar user/menu controls for Workbench +188/-0

Add top-bar user/menu controls for Workbench

• Adds a Workbench user/menu component for session controls and navigation affordances. Designed to keep the shell consistent across all screens.

frontend/components/workbench/UserMenu.tsx

Dashboard.tsxAdd Dashboard screen with stats and recent activity table +114/-0

Add Dashboard screen with stats and recent activity table

• Implements the Workbench dashboard view showing headline stats and a recent scans/activity table. Provides navigation hooks into history/evidence workflows.

frontend/components/workbench/screens/Dashboard.tsx

Evidence.tsxAdd Evidence screen with YARA findings and bounded disassembly +331/-0

Add Evidence screen with YARA findings and bounded disassembly

• Implements the evidence explorer including behavior signals, session metadata, YARA finding display, and a UI flow to request bounded static disassembly from the API. Fetches session history and provides drill-down into details without overwhelming the UI.

frontend/components/workbench/screens/Evidence.tsx

LiveMonitor.tsxAdd Live Monitor screen consuming orchestrator WebSockets with fallback polling +246/-0

Add Live Monitor screen consuming orchestrator WebSockets with fallback polling

• Adds a live monitoring view that connects to the orchestrator WebSocket feed and appends structured event lines. Includes reconnect/backoff handling and falls back to polling /api/scans when the orchestrator is offline.

frontend/components/workbench/screens/LiveMonitor.tsx

ScanHistory.tsxAdd Scan History screen with HTTPQL search and receipt preview +128/-0

Add Scan History screen with HTTPQL search and receipt preview

• Implements a scan history list/detail view with structured query parsing and a raw receipt viewer. Fetches scan sessions from /api/scans and supports quick triage by verdict/type/sha256.

frontend/components/workbench/screens/ScanHistory.tsx

Settings.tsxAdd Settings screen for API key, service status, and model retraining +190/-0

Add Settings screen for API key, service status, and model retraining

• Implements settings for storing an API key locally, checking connectivity to API/Ollama/orchestrator, displaying model info, and triggering background retraining. Provides user-facing affordances for the new /api/model/* endpoints.

frontend/components/workbench/screens/Settings.tsx

Signatures.tsxAdd Signatures editor with validation/save flow +123/-0

Add Signatures editor with validation/save flow

• Implements a YARA signatures editor that reads/writes rules via /api/signatures and surfaces validation errors. Enables local iteration on the signature pack without modifying files manually.

frontend/components/workbench/screens/Signatures.tsx

search.tsAdd query parser and row matcher for structured filtering +68/-0

Add query parser and row matcher for structured filtering

• Adds a minimal HTTPQL-like tokenizer/parser and a row matching function with wildcard support and free-text fallback. Used by ScanHistory to filter locally fetched scan rows.

frontend/components/workbench/search.ts

base.pyIntroduce analyzer interface and finding/action primitives +64/-0

Introduce analyzer interface and finding/action primitives

• Adds SampleAnalyzer base contract plus AnalysisFinding and AnalyzerActions for emitting structured findings. Documents ordering constraints required to build a complete graph before ML classification.

monitor/analyzers/base.py

legs_analyzer.pyAdd LegsAnalyzer wrapper around AIMeshOrchestrator triage +62/-0

Add LegsAnalyzer wrapper around AIMeshOrchestrator triage

• Introduces a thin wrapper that runs the AI triage stages via AIMeshOrchestrator and emits a structured finding. Designed for Phase A parity and explicitly requires a live Ollama environment.

monitor/analyzers/legs_analyzer.py

rf_analyzer.pyAdd RandomForestAnalyzer wrapper for detect_anomaly() +33/-0

Add RandomForestAnalyzer wrapper for detect_anomaly()

• Wraps the ML detector call and emits a structured finding including verdict, confidence (probability), and reasons. Intended to keep wrapper output identical to direct detect_anomaly behavior.

monitor/analyzers/rf_analyzer.py

static_disassembly_analyzer.pyAdd bounded radare2 disassembly analyzer and helper function +198/-0

Add bounded radare2 disassembly analyzer and helper function

• Introduces a safe, bounded static disassembly utility using r2pipe with file-size and instruction caps plus basic binary magic checks. Provides an analyzer wrapper that emits annotated evidence for UI consumption without classifying.

monitor/analyzers/static_disassembly_analyzer.py

syscall_analyzer.pyAdd SyscallAnalyzer wrapper for parsing + signature matching +37/-0

Add SyscallAnalyzer wrapper for parsing + signature matching

• Wraps parse_strace_log and signature matching into a single analyzer producing parsed output and signature matches. Designed to feed graph building before ML classification per the analyzer ordering contract.

monitor/analyzers/syscall_analyzer.py

yara_analyzer.pyAdd YaraAnalyzer wrapper for scan_with_yara() +40/-0

Add YaraAnalyzer wrapper for scan_with_yara()

• Adds an analyzer wrapper that runs YARA scanning across logs and optional extracted package directories and emits a structured finding. Computes a simple confidence heuristic based on match count and severity.

monitor/analyzers/yara_analyzer.py

cve_sync.pyAdd CVE threat intel sync and audit trail persistence +488/-0

Add CVE threat intel sync and audit trail persistence

• Introduces CVE sync logic pulling from NVD v2, CISA KEV, and OSV with response-size caps and prompt-injection-aware description truncation. Persists an audit trail under .tracetree/cve_audit and supports background auto-sync for CLI-driven workflows.

monitor/cve_sync.py

yara.pyEnhance YARA scanning with offsets, log exclusions, and new rules +52/-12

Enhance YARA scanning with offsets, log exclusions, and new rules

• Excludes .log files to avoid scanning diagnostic strace logs, adds matched byte offsets to results for downstream static analysis, and introduces/adjusts rules (including a provisional PowerShell dropper rule). Keeps backward compatibility via matched_strings while enabling disassembly-at-offset workflows.

monitor/yara.py

retrain_model.pyAdd standalone model retraining script producing skops + metrics +153/-0

Add standalone model retraining script producing skops + metrics

• Introduces a retraining pipeline that reads labeled fixtures, trains/evaluates a RandomForest model, writes ml/model.skops, and emits ml/metrics.json. Intended to be triggered from the Workbench settings API endpoint.

retrain_model.py

sandbox.pySupport MCP target type and quarantine extracted binaries for static analysis +27/-2

Support MCP target type and quarantine extracted binaries for static analysis

• Extends pip sandbox behavior to also cover MCP targets and adds a quarantine extraction step for zip-malware artifacts before container teardown. Writes a .qdir sidecar for downstream YARA and disassembly tooling to locate retained binaries safely.

sandbox/sandbox.py

Bug fix (4) +158 / -56
detector.pyAdjust anomaly detector semantics to support new wrappers/UI +5/-1

Adjust anomaly detector semantics to support new wrappers/UI

• Small updates to detection output/semantics to interoperate with the new RandomForestAnalyzer wrapper and UI expectations (confidence/probability handling). Keeps backward compatibility while exposing richer fields for downstream consumers.

ml/detector.py

parser.pyReduce false positives via refined severity weighting and wrapper filtering +148/-55

Reduce false positives via refined severity weighting and wrapper filtering

• Rebalances syscall severity defaults (especially network/file/memory) and introduces additional benign path heuristics to prevent benign pip/install activity from accumulating severity. Filters failed execve path searches and infrastructure/root-wrapper activity to avoid pushing scans over hard thresholds spuriously.

monitor/parser.py

signatures.pyAvoid treating failed execve events as non-standard binaries +3/-0

Avoid treating failed execve events as non-standard binaries

• Updates signature matching conditions to ignore execve events marked as failed. Prevents sequence-based signatures from triggering on PATH-search noise and other harmless failures.

monitor/signatures.py

utils.pyExpand benign binary allowlist for common tooling +2/-0

Expand benign binary allowlist for common tooling

• Adds additional known benign binaries (e.g., timeout, wine64) to reduce noisy flags during analysis. Helps align parser/signature behavior with real-world environments.

monitor/utils.py

Refactor (1) +0 / -7
tracetree_mcp_server.pyRemove obsolete MCP server content +0/-7

Remove obsolete MCP server content

• Removes a small amount of MCP server code/stub to keep the hackathon branch focused on the Workbench path. Reduces confusion around the current supported MCP workflow shape.

tracetree_mcp_server.py

Tests (2) +177 / -2
test_anomaly_semantics.pyAlign anomaly semantics tests with updated detector behavior +2/-2

Align anomaly semantics tests with updated detector behavior

• Updates unit assertions to match refined AnomalyVerdict semantics/confidence handling. Keeps regression coverage for detector output shape and meaning.

tests/unit/test_anomaly_semantics.py

test_phase_a_wrappers.pyAdd parity tests for Phase A analyzer wrappers +175/-0

Add parity tests for Phase A analyzer wrappers

• Adds unit tests asserting wrapper analyzer raw_output/verdicts match direct function calls for YARA scanning, syscall parsing/signatures, and RF classification. Skips LegsAnalyzer due to external Ollama dependency.

tests/unit/test_phase_a_wrappers.py

Documentation (1) +400 / -0
CODEX_UI_PROMPT.mdAdd Codex implementation spec for Workbench UI +400/-0

Add Codex implementation spec for Workbench UI

• Introduces a detailed design and implementation prompt describing the Workbench look-and-feel, constraints, and token system. Serves as the source-of-truth for dense, square-corner, analyst-oriented UI styling.

docs/CODEX_UI_PROMPT.md

Other (5) +80 / -118
globals.cssReplace retro utility styles with Workbench design tokens +48/-100

Replace retro utility styles with Workbench design tokens

• Adds CSS variables for a dark workbench theme (backgrounds, borders, typography, severity colors) and enforces square corners and no shadows globally. Simplifies global styles and adds scrollbar/selection styling aligned with the spec.

frontend/app/globals.css

tailwind.config.tsMirror design tokens into Tailwind theme extensions +30/-18

Mirror design tokens into Tailwind theme extensions

• Updates Tailwind content paths and adds theme extensions mapping CSS variables to named colors and fonts. Aligns Tailwind utilities with the Workbench token model.

frontend/tailwind.config.ts

__init__.pyPackage marker for analyzers module +0/-0

Package marker for analyzers module

• Ensures the analyzers directory is treated as a package for imports and test discovery. No functional behavior change expected.

monitor/analyzers/init.py

pyproject.tomlProject metadata tweak for new tooling surface +1/-0

Project metadata tweak for new tooling surface

• Small configuration change to support the expanded tooling/scripts introduced by the Workbench feature. Likely aligns packaging/build behavior with new modules.

pyproject.toml

setup.pyPackaging tweak for new CLI/tools exposure +1/-0

Packaging tweak for new CLI/tools exposure

• Small setup configuration update to account for new modules/scripts added in this PR. Ensures installs remain functional as the feature surface expands.

setup.py

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

@qodo-code-review

qodo-code-review Bot commented Jul 19, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (5) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Analysis tuple arity crash 🐞 Bug ≡ Correctness
Description
cli.perform_analysis() now returns 9 values (including log_path) and callers unpack 9, but
several early error returns still return only 8 values, causing `ValueError: not enough values to
unpack` and aborting the CLI on sandbox/parser/graph failures.
Code

cli.py[R320-327]

def perform_analysis(target: str, target_type: str, progress, console, workspace_root: str = None, controlled_network: bool = False) -> Tuple[bool, float, dict, dict, list, list, list, dict, str]:
    """Helper to run the full sandbox → parse → graph → ML pipeline for a single target.

    Returns:
        (is_malicious, confidence, graph_data, parsed_data, signature_matches, temporal_patterns, yara_matches, ngram_data, log_path)
    """
+    _purge_stale_quarantine()
    from sandbox.sandbox import run_sandbox
Relevance

⭐⭐⭐ High

Team fixes deterministic runtime crashes in analysis pipeline changes (expanded returns in PR50).

PR-#50

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The function signature/docstring and callers expect 9 values, but early failure returns provide only
8, which will deterministically crash at the unpack sites when those branches execute.

cli.py[320-366]
cli.py[415-431]
cli.py[538-546]
cli.py[1244-1247]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`perform_analysis()` was expanded to return 9 values (now including `log_path`), and callers were updated to unpack 9 values. However, multiple early `return` statements still return 8 values, which will raise during unpack and crash the CLI on common failure paths.

### Issue Context
This affects error handling for sandbox failure, parser failure, and graph-building failure; ML failure was already updated to return 9 values.

### Fix Focus Areas
- cli.py[320-431]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Tar extractall filter unsupported ✓ Resolved 🐞 Bug ≡ Correctness
Description
sandbox.run_sandbox() uses tarfile.extractall(..., filter="data") when quarantining zip-malware
extracts; since the project supports Python >=3.9, this will raise TypeError on Python 3.9–3.11
and silently disable quarantine extraction/static binary analysis.
Code

sandbox/sandbox.py[R644-647]

+                    with tarfile.open(q_tar_tmp.name) as qtar:
+                        # nosec — extraction confined to quarantine_dir, no symlink follow
+                        qtar.extractall(path=str(quarantine_dir), filter="data")
+                    os.remove(q_tar_tmp.name)
Relevance

⭐⭐ Medium

Sandbox reliability suggestions often accepted (PR41), but no precedent on tarfile filter kw
Python-compat.

PR-#41

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The code calls extractall(..., filter="data") while the project explicitly supports Python 3.9+,
where this keyword is not available; the exception is caught and logged as non-fatal, meaning
extraction will be skipped on those runtimes.

sandbox/sandbox.py[630-654]
pyproject.toml[5-12]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`tarfile.TarFile.extractall(filter=...)` is not available on Python 3.9–3.11, but the repo declares `requires-python = >=3.9`. The current quarantine extraction path will throw and be caught as non-fatal, resulting in missing quarantined binaries (and downstream static disassembly / file scanning gaps).

### Issue Context
This runs only for `target_type == "zip-malware"` and is intended to retain extracted files under `.tracetree/quarantine/`.

### Fix Focus Areas
- sandbox/sandbox.py[630-654]
- pyproject.toml[5-12]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. YARA skipped for pip 🐞 Bug ≡ Correctness
Description
monitor.yara._collect_files() now excludes .log files (including strace logs), and
cli.perform_analysis() passes only log_path for pip analyses (no package_dir), so YARA
scanning returns an empty result for pip targets and the YARA analyzer reports BENIGN.
Code

monitor/yara.py[R459-462]

    if log_path and Path(log_path).exists():
-        files.append(Path(log_path))
+        lp = Path(log_path)
+        if lp.suffix.lower() not in SCAN_EXCLUDE_EXTENSIONS:
+            files.append(lp)
Relevance

⭐⭐ Medium

YARA behavior/exclusions adjusted before (PR37), but no direct precedent for scanning .log for pip.

PR-#37

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The YARA collector explicitly filters out .log for the provided log_path, and the CLI passes
package_dir=None for pip (no quarantine sidecar), leaving nothing to scan.

monitor/yara.py[23-28]
monitor/yara.py[452-463]
cli.py[386-392]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
YARA scanning now skips any `log_path` with suffix `.log`. TraceTree’s strace outputs are `*_strace.log`, so for normal pip analyses (where no `package_dir` is provided), the YARA scan has zero files to scan and always returns `[]`.

### Issue Context
The intent comment suggests avoiding false positives from scanning logs, but the current wiring provides no alternative artifact path for pip targets.

### Fix Focus Areas
- monitor/yara.py[23-28]
- monitor/yara.py[452-463]
- cli.py[386-392]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. Quarantine TTL env crashes 🐞 Bug ☼ Reliability
Description
_purge_stale_quarantine() parses TRACETREE_QUARANTINE_TTL_DAYS with int() without handling
ValueError, so a malformed env var will crash every analysis before it starts.
Code

cli.py[R297-307]

+def _purge_stale_quarantine(max_age_days: int = 7) -> None:
+    """Delete quarantine session dirs older than max_age_days.
+
+    Runs silently at CLI startup — quarantine holds live malware binaries and
+    must not accumulate indefinitely. 7-day default suits single-user local use.
+    Users who need longer retention can set TRACETREE_QUARANTINE_TTL_DAYS env var.
+    """
+    import shutil as _shutil
+    import time as _time
+    ttl = int(os.environ.get("TRACETREE_QUARANTINE_TTL_DAYS", max_age_days))
+    quarantine_root = Path.cwd() / ".tracetree" / "quarantine"
Relevance

⭐⭐⭐ High

Team accepted crash-prevention by catching ValueError/TypeError in parsing paths (PR41, PR14).

PR-#41
PR-#14

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The TTL is parsed with an unguarded int() and the purge runs at analysis start, so a malformed env
var prevents analysis from running at all.

cli.py[297-327]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`_purge_stale_quarantine()` converts `TRACETREE_QUARANTINE_TTL_DAYS` via `int(...)` without validation. Any non-integer value (including empty string) raises `ValueError` and aborts CLI runs.

### Issue Context
This function is invoked unconditionally at the start of `perform_analysis()`, so the crash happens before sandboxing.

### Fix Focus Areas
- cli.py[297-327]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. radare2 session leak 🐞 Bug ☼ Reliability
Description
disassemble_at() opens an r2pipe session but does not call r2.quit() if an exception occurs
after r2pipe.open(), leaking radare2 processes/file descriptors under error conditions.
Code

monitor/analyzers/static_disassembly_analyzer.py[R103-135]

+    try:
+        r2 = r2pipe.open(file_path, flags=["-2"])
+
+        resolved = _resolve_offset(r2, offset, file_path)
+        if resolved is None:
+            result["error"] = "could not resolve a disassembly offset (no symbols, no YARA offset)"
+            r2.quit()
+            return result
+
+        result["offset_used"] = resolved
+
+        # Request only the bounded instruction window. Do not run whole-program
+        # analysis or request complete function JSON for untrusted binaries.
+        pdj_raw = r2.cmd(f"pdj {max_insns} @ {resolved}")
+        if pdj_raw.strip():
+            try:
+                insns = json.loads(pdj_raw)
+                result["instructions"] = [
+                    {
+                        "offset": insn.get("offset", resolved),
+                        "disasm": insn.get("disasm", ""),
+                        "type": insn.get("type", ""),
+                        "bytes": insn.get("bytes", ""),
+                    }
+                    for insn in insns[:max_insns]
+                ]
+            except json.JSONDecodeError as e:
+                result["error"] = f"pdj parse error: {e}"
+
+        r2.quit()
+    except Exception as e:
+        result["error"] = str(e)
+
Relevance

⭐⭐ Medium

File path not found in repo history; no evidence of r2pipe cleanup enforcement.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The exception handler after opening r2 does not perform cleanup, so failures after session
creation can leave radare2 running.

monitor/analyzers/static_disassembly_analyzer.py[103-136]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
If `_resolve_offset()` or `r2.cmd(...)` raises after `r2pipe.open(...)`, the broad `except Exception` sets `result["error"]` and returns without closing the r2 session.

### Issue Context
Normal completion and one early-return path call `r2.quit()`, but the error path does not.

### Fix Focus Areas
- monitor/analyzers/static_disassembly_analyzer.py[103-136]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. SSE CLI process orphaning 🐞 Bug ☼ Reliability
Description
POST /api/execute spawns a CLI subprocess and streams output, but on client
disconnect/cancellation it only closes the PTY FD and never terminates the subprocess, allowing
long-running scans to continue headless and exhaust host/Docker resources.
Code

api/main.py[R283-341]

        proc = subprocess.Popen(
            argv,
            shell=False,
-            stdout=subprocess.PIPE,
-            stderr=subprocess.STDOUT,
-            text=True,
-            bufsize=1,
+            stdin=subprocess.DEVNULL,
+            stdout=slave_fd,
+            stderr=slave_fd,
+            close_fds=True,
            env=env,
        )
+        os.close(slave_fd)
+
+        # Set master non-blocking
+        fl = fcntl.fcntl(master_fd, fcntl.F_GETFL)
+        fcntl.fcntl(master_fd, fcntl.F_SETFL, fl | os.O_NONBLOCK)
+
+        buf = ""
+        last_output = time.time()
+
+        try:
+            while True:
+                ready, _, _ = select.select([master_fd], [], [], 0.1)
+                if ready:
+                    try:
+                        chunk = os.read(master_fd, 4096)
+                        if not chunk:
+                            break
+                        last_output = time.time()
+                        text = chunk.decode("utf-8", errors="replace")
+                        # Collapse carriage returns: spinner emits \r to overwrite the
+                        # current line in a real terminal. Split on \n first, then for
+                        # each line keep only the LAST frame after any \r so the browser
+                        # sees only the final settled state, not every intermediate frame.
+                        text = text.replace("\r\n", "\n")
+                        segments = text.split("\n")
+                        segments = [s.split("\r")[-1] for s in segments]
+                        text = "\n".join(segments)
+                        buf += text
+                        while "\n" in buf:
+                            line, buf = buf.split("\n", 1)
+                            clean = _strip_ansi(line).rstrip()
+                            if clean:
+                                yield f"data: {clean}\n\n"
+                    except OSError:
+                        break
+                else:
+                    if proc.poll() is not None:
+                        break
+                    # Send keepalive dot so user sees it's running
+                    if time.time() - last_output > 3:
+                        yield "data: ...\n\n"
+                        last_output = time.time()
+        finally:
+            try:
+                os.close(master_fd)
+            except OSError:
+                pass

-        for line in iter(proc.stdout.readline, ""):
-            yield f"data: {line}\n\n"
-
-        proc.stdout.close()
        proc.wait()
-        yield f"data: \n--- PROCESS FINISHED WITH CODE {proc.returncode} ---\n\n"
+        yield f"data: --- FINISHED (exit {proc.returncode}) ---\n\n"
Relevance

⭐⭐ Medium

No prior reviews on /api/execute disconnect cleanup; team hardens endpoint often (PR46).

PR-#46

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The generator’s finally block only closes master_fd; there is no proc.terminate()/kill() on
generator close, and the code waits for process completion regardless of client state.

api/main.py[272-341]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The SSE generator cleans up file descriptors but never terminates the spawned subprocess when the streaming response is cancelled (e.g., browser tab closed). This can leave expensive analyses running without any client.

### Issue Context
The subprocess is started via `subprocess.Popen(...)` and the generator does `proc.wait()` unconditionally after the loop.

### Fix Focus Areas
- api/main.py[272-342]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Qodo Logo

Comment thread cli.py
Comment on lines 320 to 327
def perform_analysis(target: str, target_type: str, progress, console, workspace_root: str = None, controlled_network: bool = False) -> Tuple[bool, float, dict, dict, list, list, list, dict, str]:
"""Helper to run the full sandbox → parse → graph → ML pipeline for a single target.

Returns:
(is_malicious, confidence, graph_data, parsed_data, signature_matches, temporal_patterns, yara_matches, ngram_data, log_path)
"""
_purge_stale_quarantine()
from sandbox.sandbox import run_sandbox

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Analysis tuple arity crash 🐞 Bug ≡ Correctness

cli.perform_analysis() now returns 9 values (including log_path) and callers unpack 9, but
several early error returns still return only 8 values, causing `ValueError: not enough values to
unpack` and aborting the CLI on sandbox/parser/graph failures.
Agent Prompt
### Issue description
`perform_analysis()` was expanded to return 9 values (now including `log_path`), and callers were updated to unpack 9 values. However, multiple early `return` statements still return 8 values, which will raise during unpack and crash the CLI on common failure paths.

### Issue Context
This affects error handling for sandbox failure, parser failure, and graph-building failure; ML failure was already updated to return 9 values.

### Fix Focus Areas
- cli.py[320-431]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread cli.py
Comment on lines +297 to +307
def _purge_stale_quarantine(max_age_days: int = 7) -> None:
"""Delete quarantine session dirs older than max_age_days.

Runs silently at CLI startup — quarantine holds live malware binaries and
must not accumulate indefinitely. 7-day default suits single-user local use.
Users who need longer retention can set TRACETREE_QUARANTINE_TTL_DAYS env var.
"""
import shutil as _shutil
import time as _time
ttl = int(os.environ.get("TRACETREE_QUARANTINE_TTL_DAYS", max_age_days))
quarantine_root = Path.cwd() / ".tracetree" / "quarantine"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Quarantine ttl env crashes 🐞 Bug ☼ Reliability

_purge_stale_quarantine() parses TRACETREE_QUARANTINE_TTL_DAYS with int() without handling
ValueError, so a malformed env var will crash every analysis before it starts.
Agent Prompt
### Issue description
`_purge_stale_quarantine()` converts `TRACETREE_QUARANTINE_TTL_DAYS` via `int(...)` without validation. Any non-integer value (including empty string) raises `ValueError` and aborts CLI runs.

### Issue Context
This function is invoked unconditionally at the start of `perform_analysis()`, so the crash happens before sandboxing.

### Fix Focus Areas
- cli.py[297-327]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread sandbox/sandbox.py
Comment thread monitor/yara.py
Comment on lines 459 to +462
if log_path and Path(log_path).exists():
files.append(Path(log_path))
lp = Path(log_path)
if lp.suffix.lower() not in SCAN_EXCLUDE_EXTENSIONS:
files.append(lp)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

4. Yara skipped for pip 🐞 Bug ≡ Correctness

monitor.yara._collect_files() now excludes .log files (including strace logs), and
cli.perform_analysis() passes only log_path for pip analyses (no package_dir), so YARA
scanning returns an empty result for pip targets and the YARA analyzer reports BENIGN.
Agent Prompt
### Issue description
YARA scanning now skips any `log_path` with suffix `.log`. TraceTree’s strace outputs are `*_strace.log`, so for normal pip analyses (where no `package_dir` is provided), the YARA scan has zero files to scan and always returns `[]`.

### Issue Context
The intent comment suggests avoiding false positives from scanning logs, but the current wiring provides no alternative artifact path for pip targets.

### Fix Focus Areas
- monitor/yara.py[23-28]
- monitor/yara.py[452-463]
- cli.py[386-392]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread api/main.py
Comment on lines 283 to +341
proc = subprocess.Popen(
argv,
shell=False,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
text=True,
bufsize=1,
stdin=subprocess.DEVNULL,
stdout=slave_fd,
stderr=slave_fd,
close_fds=True,
env=env,
)
os.close(slave_fd)

# Set master non-blocking
fl = fcntl.fcntl(master_fd, fcntl.F_GETFL)
fcntl.fcntl(master_fd, fcntl.F_SETFL, fl | os.O_NONBLOCK)

buf = ""
last_output = time.time()

try:
while True:
ready, _, _ = select.select([master_fd], [], [], 0.1)
if ready:
try:
chunk = os.read(master_fd, 4096)
if not chunk:
break
last_output = time.time()
text = chunk.decode("utf-8", errors="replace")
# Collapse carriage returns: spinner emits \r to overwrite the
# current line in a real terminal. Split on \n first, then for
# each line keep only the LAST frame after any \r so the browser
# sees only the final settled state, not every intermediate frame.
text = text.replace("\r\n", "\n")
segments = text.split("\n")
segments = [s.split("\r")[-1] for s in segments]
text = "\n".join(segments)
buf += text
while "\n" in buf:
line, buf = buf.split("\n", 1)
clean = _strip_ansi(line).rstrip()
if clean:
yield f"data: {clean}\n\n"
except OSError:
break
else:
if proc.poll() is not None:
break
# Send keepalive dot so user sees it's running
if time.time() - last_output > 3:
yield "data: ...\n\n"
last_output = time.time()
finally:
try:
os.close(master_fd)
except OSError:
pass

for line in iter(proc.stdout.readline, ""):
yield f"data: {line}\n\n"

proc.stdout.close()
proc.wait()
yield f"data: \n--- PROCESS FINISHED WITH CODE {proc.returncode} ---\n\n"
yield f"data: --- FINISHED (exit {proc.returncode}) ---\n\n"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

5. Sse cli process orphaning 🐞 Bug ☼ Reliability

POST /api/execute spawns a CLI subprocess and streams output, but on client
disconnect/cancellation it only closes the PTY FD and never terminates the subprocess, allowing
long-running scans to continue headless and exhaust host/Docker resources.
Agent Prompt
### Issue description
The SSE generator cleans up file descriptors but never terminates the spawned subprocess when the streaming response is cancelled (e.g., browser tab closed). This can leave expensive analyses running without any client.

### Issue Context
The subprocess is started via `subprocess.Popen(...)` and the generator does `proc.wait()` unconditionally after the loop.

### Fix Focus Areas
- api/main.py[272-342]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +103 to +135
try:
r2 = r2pipe.open(file_path, flags=["-2"])

resolved = _resolve_offset(r2, offset, file_path)
if resolved is None:
result["error"] = "could not resolve a disassembly offset (no symbols, no YARA offset)"
r2.quit()
return result

result["offset_used"] = resolved

# Request only the bounded instruction window. Do not run whole-program
# analysis or request complete function JSON for untrusted binaries.
pdj_raw = r2.cmd(f"pdj {max_insns} @ {resolved}")
if pdj_raw.strip():
try:
insns = json.loads(pdj_raw)
result["instructions"] = [
{
"offset": insn.get("offset", resolved),
"disasm": insn.get("disasm", ""),
"type": insn.get("type", ""),
"bytes": insn.get("bytes", ""),
}
for insn in insns[:max_insns]
]
except json.JSONDecodeError as e:
result["error"] = f"pdj parse error: {e}"

r2.quit()
except Exception as e:
result["error"] = str(e)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

6. Radare2 session leak 🐞 Bug ☼ Reliability

disassemble_at() opens an r2pipe session but does not call r2.quit() if an exception occurs
after r2pipe.open(), leaking radare2 processes/file descriptors under error conditions.
Agent Prompt
### Issue description
If `_resolve_offset()` or `r2.cmd(...)` raises after `r2pipe.open(...)`, the broad `except Exception` sets `result["error"]` and returns without closing the r2 session.

### Issue Context
Normal completion and one early-return path call `r2.quit()`, but the error path does not.

### Fix Focus Areas
- monitor/analyzers/static_disassembly_analyzer.py[103-136]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 20

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
data/mcp_rce_signatures.yara (2)

68-80: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

MCP_UI_CSS_Exfiltration condition now fires on a benign, ubiquitous CSS pattern alone.

Condition changed from requiring $css_import_url to any of them, so $css_attribute_selector (/\[class\s*[\^$*]?=\s*["'][^"']*["']\s*\]/, matching ordinary attribute selectors like [class*="foo"]) alone now triggers a "high" severity CSS exfiltration finding. This pattern is extremely common in normal CSS/HTML and unrelated to exfiltration, which real exfiltration requires (@import url(https://...)). This will generate false positives at high severity.

🐛 Proposed fix — require the actual exfiltration indicator
     condition:
-        any of them
+        $css_import_url or ($css_attribute_selector and $css_import_url)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@data/mcp_rce_signatures.yara` around lines 68 - 80, Update the condition in
rule MCP_UI_CSS_Exfiltration to require the $css_import_url indicator instead of
matching any string, while preserving the existing detection strings and
metadata.

22-36: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Greedy $html_script_tag regex over-matches across unrelated <script> blocks.

/<script[^>]*>[\s\S]*<\/script>/ greedily spans from the first <script> to the last </script> in the scanned content, conflating unrelated script blocks into one oversized match and inflating matched_strings/matched_offsets (per monitor/yara.py's scan_with_yara contract) for files with multiple script tags. Consider a non-greedy/bounded variant to keep matches scoped to a single tag.

♻️ Proposed fix
-        $html_script_tag = /<script[^>]*>[\s\S]*<\/script>/ nocase
+        $html_script_tag = /<script[^>]*>[\s\S]{0,4096}?<\/script>/ nocase
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@data/mcp_rce_signatures.yara` around lines 22 - 36, Update the
`$html_script_tag` pattern in `MCP_UI_HTML_Injection` to use a non-greedy or
otherwise bounded body match, so each match spans only one
`<script>...</script>` block and does not combine unrelated script tags.
Preserve the existing case-insensitive detection and surrounding tag matching.
ml/detector.py (1)

389-398: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Normalize RF confidence to the shared 0–1 scale.
monitor/analyzers/base.py defines AnalysisFinding.confidence as 0.0–1.0, and the other analyzers already follow that contract. RandomForestAnalyzer still forwards ml_probability directly, so RF findings come out on a 0–100 scale while the fallback path divides by 100. Normalize the RF branch here, and update the wrapper test if needed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ml/detector.py` around lines 389 - 398, Normalize the RandomForestClassifier
confidence in the RF branch of RandomForestAnalyzer by converting the
percentage-based ml_probability to the shared 0.0–1.0 scale before assigning
ml_confidence. Keep the malicious-probability calculation unchanged, ensure the
fallback path remains consistent, and update the wrapper test expectations if
they assert the old 0–100 value.
🧹 Nitpick comments (4)
frontend/app/page.tsx (1)

291-294: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant verdict re-normalization.

verdictToScanRow (Lines 86-96) already coerces verdict into the allowed enum before rows enter scan_history; re-validating it again here is unnecessary duplication.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/app/page.tsx` around lines 291 - 294, Remove the redundant verdict
validation and cast from the recentScans mapping, relying on verdictToScanRow to
provide the normalized ScanRow verdict. Preserve the existing scan_history
slicing and object spreading while assigning r.verdict directly.
monitor/parser.py (2)

450-484: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant condition, but logic is otherwise sound.

is_benign = _is_benign_binary(target_bin) or is_root_wrapper or is_failed already folds in is_root_wrapper, so the subsequent if not is_benign and not is_root_wrapper: (line 464) has a redundant and not is_root_wrapper clause (it can never be false there since is_root_wrapper being true would already make is_benign true). Harmless but confusing.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@monitor/parser.py` around lines 450 - 484, Remove the redundant “and not
is_root_wrapper” condition from the suspicious-binary check in the execve
handling block, since is_benign already includes is_root_wrapper. Keep the
existing severity update and suspicious_flags behavior unchanged.

590-602: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor: sendto's emitted severity is hardcoded rather than derived from severity.

severity = max(severity, 2.0) is computed but then _make_event(syscall, "AF_INET", 2.0, {}) hardcodes 2.0 directly instead of passing severity. Functionally equivalent today (base weight is 0.0), but if SEVERITY_WEIGHTS["sendto"] is ever raised, total_severity and the recorded event severity would silently diverge.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@monitor/parser.py` around lines 590 - 602, Update the sendto branch in the
parser’s syscall handling to pass the computed severity value into _make_event
instead of hardcoding 2.0. Keep the existing severity = max(severity, 2.0)
calculation and AF_INET behavior unchanged so recorded event severity stays
aligned with total severity if the base weight changes.
tests/unit/test_phase_a_wrappers.py (1)

169-175: 🚀 Performance & Scalability | 🔵 Trivial

test_confidence_is_ml_probability may be testing a cache hit, not independent computation.

detect_anomaly memoizes results in _PREDICTION_CACHE keyed by a feature hash. This calls RandomForestAnalyzer().analyze() (which internally calls detect_anomaly) and then calls detect_anomaly again directly with the same inputs, so the second call likely returns the same cached AnomalyVerdict object rather than an independently recomputed one, weakening this as a regression test for the confidence-derivation logic itself.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_phase_a_wrappers.py` around lines 169 - 175, The
test_confidence_is_ml_probability currently risks comparing two references to
the same cached AnomalyVerdict from _PREDICTION_CACHE. Update the test to obtain
an independently computed expected verdict, such as by clearing or bypassing the
prediction cache before the direct detect_anomaly call, then derive
expected_conf from that result while preserving the existing confidence
assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@api/main.py`:
- Around line 604-616: Update the Docker status request used by the UserMenu
component to include the required X-API-Key header so it satisfies
verify_api_key and avoids a 422 response; alternatively, explicitly make
docker_status public only if that is the intended access policy.

In `@cli.py`:
- Line 429: Update all early-return branches in perform_analysis, including the
sandbox-failure, parser-failure, and graph-failure paths, to return the same
9-element tuple as the successful and existing updated returns. Preserve each
branch’s current fallback values, add the missing ngram_data and trailing
log_path fields in the correct order, and ensure callers can consistently unpack
the result.

In `@frontend/app/page.tsx`:
- Around line 151-242: Update the WebSocket useEffect containing the
ai_summary_completed handler so it does not retain the initial
refreshHistory/apiKey closure. Prefer matching LiveMonitor.tsx by adding apiKey
to the effect dependencies, allowing the socket handler to rebind when the key
changes; preserve the existing cleanup and reconnect behavior.

In `@frontend/components/workbench/CommandConsole.tsx`:
- Around line 64-76: Convert each OSV severity vector to a numeric CVSS score
before assigning CVEResult.score, using the existing CVSS parsing/conversion
utility. Apply this in both mapper sites at
frontend/components/workbench/CommandConsole.tsx lines 64-76 and 174-183, while
preserving the existing fallback behavior when severity data is missing.

In `@frontend/components/workbench/DataTable.tsx`:
- Line 55: Update the comparison in the table sorting logic to pass the numeric
option to String.localeCompare, ensuring numeric-looking values such as IDs and
confidence scores sort numerically while preserving text sorting behavior.

In `@frontend/components/workbench/screens/Evidence.tsx`:
- Around line 261-268: Update the offset rendering in the
disasmResult.instructions mapping to check explicitly for a missing offset
rather than relying on truthiness, so a valid ins.offset of 0 displays as 0x0
while genuinely absent offsets remain blank.

In `@frontend/components/workbench/screens/ScanHistory.tsx`:
- Around line 108-115: Update the findings rendering in ScanHistory’s RawWell
props to handle JSON.parse failures without throwing during React rendering.
Safely parse selected.findings and fall back to the existing
target/verdict/confidence/SHA256 lines when the value is malformed or not valid
JSON, while preserving the formatted JSON output for valid findings.

In `@frontend/components/workbench/search.ts`:
- Around line 52-54: Update the token matching logic around tokVal and regex to
escape all user-provided regular-expression metacharacters before translating *
into the intended wildcard pattern, while preserving wildcard behavior. Wrap
RegExp construction and testing in try-catch so invalid input cannot escape the
render path; treat failures as non-matches and keep rendering stable.

In `@frontend/components/workbench/UserMenu.tsx`:
- Around line 114-117: Update the API status row in UserMenu to use an apiOnline
prop instead of the hardcoded dot(true), and add the corresponding prop to the
component’s type/signature. In page.tsx, pass apiOnline={stats.api_online} when
rendering UserMenu so the indicator reflects the tracked gateway health.
- Around line 16-23: Update fetchDocker in UserMenu to include the configured
X-API-Key when requesting /api/docker/status, preserving the existing
unavailable fallback on failure. Then update the menu’s API status rendering to
pass the fetched Docker/health state to dot instead of hardcoding true, so the
indicator reflects the real backend status.

In `@monitor/analyzers/rf_analyzer.py`:
- Around line 17-33: Update the confidence calculation in analyze to normalize
result.ml_probability from its documented 0–100 scale to 0.0–1.0 by dividing by
100.0, matching the existing risk_score fallback while preserving the None
handling.

In `@monitor/analyzers/static_disassembly_analyzer.py`:
- Around line 103-134: Ensure the radare2 process opened in the disassembly
analysis flow is always closed by moving cleanup for r2.quit() into a finally
block surrounding _resolve_offset and the pdj command processing. Preserve the
existing early return and exception error handling, while avoiding duplicate
quit calls on every exit path.

In `@monitor/analyzers/syscall_analyzer.py`:
- Around line 25-27: Update the confidence calculation near SyscallAnalyzer’s
verdict logic so any non-empty sig_matches producing a MALICIOUS verdict cannot
report zero confidence; derive confidence from the signature-driven malicious
condition as well as severity, while preserving the existing severity-based cap
and behavior for non-signature findings.

In `@monitor/cve_sync.py`:
- Around line 246-301: Preserve CVEs beyond the AI batch in the success path of
the function containing `batch` and `parsed`: apply enrichment only to the first
20 entries, then return those enriched entries combined with the untouched
remainder of `cves`. Keep the existing failure behavior and ensure the returned
list retains the original CVE order and full length.

In `@monitor/parser.py`:
- Around line 296-297: Harden _is_benign_path and the is_pip_internal check so
BENIGN_PATH_SUBSTRINGS entries match only complete path components or validated
path prefixes, never arbitrary substrings within a filename or directory name.
Preserve legitimate libc.so, ld.so, locale, distutils, and pip/_internal
exemptions while ensuring crafted paths such as payload_libc.so_dropper cannot
suppress sensitive-file detection.
- Around line 539-568: Update the mmap/mprotect event construction in the
syscall parsing block so details["has_prot_exec"] stores has_prot_exec rather
than is_rwx. Also build the event target/flags string from the protection-flags
argument that contains PROT_EXEC, instead of the first address token, so the
_check_sequence_condition fallback can detect it. Preserve the existing RWX
severity and suspicious-flag behavior.
- Around line 742-759: Fix root-wrapper cleanup in the syscall event-processing
flow so it removes an event only when the current iteration actually appended
one; do not rely on the module-level sequence_id, which may refer to an older
event. Preserve legitimate root-wrapper clone/fork/vfork events. Also add chown
to the file-access syscall tuple so it uses sensitive-path evaluation and
records severity consistently with total_severity.

In `@monitor/yara.py`:
- Around line 156-178: Remove PowerShellDropper from the production-compiled
YARA ruleset, or gate it behind the project’s existing
experimental/unvalidated-rule mechanism so scan_with_yara cannot report it
through the normal critical-severity path. Preserve the rule definition for
end-to-end verification, and ensure YaraAnalyzer does not classify its matches
as MALICIOUS until validation is complete.
- Around line 222-243: Update the YARA match processing that builds instances
and matched_offsets to support both modern StringMatch objects and legacy tuple
results from yara-python, or enforce yara-python 4.3+ through the project
dependency configuration. Preserve matched_strings and byte-offset output for
supported API shapes, ensuring scans do not fail when older results are
returned.

In `@sandbox/sandbox.py`:
- Around line 652-653: Replace the undefined log.warning call in the quarantine
extraction exception handler with the module’s in-scope console.print output
path, preserving the error message and ensuring this fallback remains non-fatal.

---

Outside diff comments:
In `@data/mcp_rce_signatures.yara`:
- Around line 68-80: Update the condition in rule MCP_UI_CSS_Exfiltration to
require the $css_import_url indicator instead of matching any string, while
preserving the existing detection strings and metadata.
- Around line 22-36: Update the `$html_script_tag` pattern in
`MCP_UI_HTML_Injection` to use a non-greedy or otherwise bounded body match, so
each match spans only one `<script>...</script>` block and does not combine
unrelated script tags. Preserve the existing case-insensitive detection and
surrounding tag matching.

In `@ml/detector.py`:
- Around line 389-398: Normalize the RandomForestClassifier confidence in the RF
branch of RandomForestAnalyzer by converting the percentage-based ml_probability
to the shared 0.0–1.0 scale before assigning ml_confidence. Keep the
malicious-probability calculation unchanged, ensure the fallback path remains
consistent, and update the wrapper test expectations if they assert the old
0–100 value.

---

Nitpick comments:
In `@frontend/app/page.tsx`:
- Around line 291-294: Remove the redundant verdict validation and cast from the
recentScans mapping, relying on verdictToScanRow to provide the normalized
ScanRow verdict. Preserve the existing scan_history slicing and object spreading
while assigning r.verdict directly.

In `@monitor/parser.py`:
- Around line 450-484: Remove the redundant “and not is_root_wrapper” condition
from the suspicious-binary check in the execve handling block, since is_benign
already includes is_root_wrapper. Keep the existing severity update and
suspicious_flags behavior unchanged.
- Around line 590-602: Update the sendto branch in the parser’s syscall handling
to pass the computed severity value into _make_event instead of hardcoding 2.0.
Keep the existing severity = max(severity, 2.0) calculation and AF_INET behavior
unchanged so recorded event severity stays aligned with total severity if the
base weight changes.

In `@tests/unit/test_phase_a_wrappers.py`:
- Around line 169-175: The test_confidence_is_ml_probability currently risks
comparing two references to the same cached AnomalyVerdict from
_PREDICTION_CACHE. Update the test to obtain an independently computed expected
verdict, such as by clearing or bypassing the prediction cache before the direct
detect_anomaly call, then derive expected_conf from that result while preserving
the existing confidence assertion.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: db0a94dd-6d5e-431f-a67b-b42cdb8c2601

📥 Commits

Reviewing files that changed from the base of the PR and between 3bec228 and 072e1d4.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (46)
  • .gitignore
  • api/main.py
  • cli.py
  • data/mcp_rce_signatures.yara
  • docs/CODEX_UI_PROMPT.md
  • frontend/app/globals.css
  • frontend/app/layout.tsx
  • frontend/app/page.tsx
  • frontend/components/MSDosPrompt.tsx
  • frontend/components/workbench/CommandConsole.tsx
  • frontend/components/workbench/DataTable.tsx
  • frontend/components/workbench/FilterBar.tsx
  • frontend/components/workbench/Panel.tsx
  • frontend/components/workbench/RawWell.tsx
  • frontend/components/workbench/SearchBar.tsx
  • frontend/components/workbench/SeverityTag.tsx
  • frontend/components/workbench/SplitPane.tsx
  • frontend/components/workbench/UserMenu.tsx
  • frontend/components/workbench/screens/Dashboard.tsx
  • frontend/components/workbench/screens/Evidence.tsx
  • frontend/components/workbench/screens/LiveMonitor.tsx
  • frontend/components/workbench/screens/ScanHistory.tsx
  • frontend/components/workbench/screens/Settings.tsx
  • frontend/components/workbench/screens/Signatures.tsx
  • frontend/components/workbench/search.ts
  • frontend/tailwind.config.ts
  • ml/detector.py
  • monitor/analyzers/__init__.py
  • monitor/analyzers/base.py
  • monitor/analyzers/legs_analyzer.py
  • monitor/analyzers/rf_analyzer.py
  • monitor/analyzers/static_disassembly_analyzer.py
  • monitor/analyzers/syscall_analyzer.py
  • monitor/analyzers/yara_analyzer.py
  • monitor/cve_sync.py
  • monitor/parser.py
  • monitor/signatures.py
  • monitor/utils.py
  • monitor/yara.py
  • pyproject.toml
  • retrain_model.py
  • sandbox/sandbox.py
  • setup.py
  • tests/unit/test_anomaly_semantics.py
  • tests/unit/test_phase_a_wrappers.py
  • tracetree_mcp_server.py
💤 Files with no reviewable changes (1)
  • tracetree_mcp_server.py

Comment thread api/main.py
Comment on lines +604 to +616
@app.get("/api/docker/status")
async def docker_status(api_key: str = Depends(verify_api_key)):
"""Check if Docker daemon is reachable."""
try:
result = subprocess.run(
["docker", "info", "--format", "{{.ServerVersion}}"],
capture_output=True, text=True, timeout=5
)
if result.returncode == 0:
return {"available": True, "version": result.stdout.strip()}
return {"available": False, "version": None, "error": result.stderr.strip()}
except (FileNotFoundError, subprocess.TimeoutExpired) as e:
return {"available": False, "version": None, "error": str(e)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -nP 'api/docker/status' frontend -A2 -B2

Repository: tejasprasad2008-afk/TraceTree

Length of output: 584


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- verify_api_key in api/main.py ---'
rg -n "def verify_api_key|verify_api_key|X-API-Key|Header\(" api/main.py -A4 -B4

echo
echo '--- UserMenu.tsx ---'
cat -n frontend/components/workbench/UserMenu.tsx | sed -n '1,120p'

echo
echo '--- X-API-Key usage in frontend ---'
rg -n '"X-API-Key"|X-API-Key|api/docker/status|fetch\(' frontend -A2 -B2

Repository: tejasprasad2008-afk/TraceTree

Length of output: 20477


/api/docker/status is behind auth, but the UI call omits X-API-Key.

verify_api_key requires a header, so frontend/components/workbench/UserMenu.tsx gets a 422 here and the Docker status badge stays offline. Add the header to this fetch or make the status route public if that’s intended.

🧰 Tools
🪛 ast-grep (0.44.1)

[error] 607-610: Command coming from incoming request
Context: subprocess.run(
["docker", "info", "--format", "{{.ServerVersion}}"],
capture_output=True, text=True, timeout=5
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)

🪛 Ruff (0.15.21)

[error] 609-609: Starting a process with a partial executable path

(S607)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@api/main.py` around lines 604 - 616, Update the Docker status request used by
the UserMenu component to include the required X-API-Key header so it satisfies
verify_api_key and avoids a 422 response; alternatively, explicitly make
docker_status public only if that is the intended access policy.

Comment thread cli.py
except Exception as e:
progress.update(task4, description=f"[bold red]✖[/] [dim]ML failed: {e}[/]")
return False, 0.0, graph_data, parsed_data, signature_matches, temporal_patterns, yara_matches, ngram_data
return False, 0.0, graph_data, parsed_data, signature_matches, temporal_patterns, yara_matches, ngram_data, log_path

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Early-return paths still return 8-tuples — caller unpack at Line 544 will ValueError.

perform_analysis now returns 9 elements (adding log_path) here and at Line 431, and the caller at Line 544 unpacks 9. But the other early returns still yield 8 elements:

  • Line 352 (sandbox failed): return False, 0.0, {}, {}, [], [], {}, {}
  • Line 365 (parser failed): same 8-tuple
  • Line 421 (graph failed): missing trailing log_path

The sandbox-failure path (Line 352) is a common outcome (e.g. Docker unavailable), so this crashes the analyze command with ValueError: not enough values to unpack (expected 9, got 8).

🐛 Proposed fix (apply to Lines 352, 365, 421)
-        return False, 0.0, {}, {}, [], [], {}, {}
+        return False, 0.0, {}, {}, [], [], [], {}, log_path
-        return False, 0.0, {}, parsed_data, signature_matches, temporal_patterns, yara_matches, ngram_data
+        return False, 0.0, {}, parsed_data, signature_matches, temporal_patterns, yara_matches, ngram_data, log_path

Note: the first two also only supply 7 collection/element slots before ngram_data; align all returns to the 9-field contract.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cli.py` at line 429, Update all early-return branches in perform_analysis,
including the sandbox-failure, parser-failure, and graph-failure paths, to
return the same 9-element tuple as the successful and existing updated returns.
Preserve each branch’s current fallback values, add the missing ngram_data and
trailing log_path fields in the correct order, and ensure callers can
consistently unpack the result.

Comment thread frontend/app/page.tsx Outdated
Comment on lines 151 to 242
// WebSocket — mirrors old page.tsx logic
useEffect(() => {
let socketTimeout: ReturnType<typeof setTimeout>;

const connectWebSocket = () => {
console.log("Connecting to Orchestrator WebSocket...");
const connect = () => {
const ws = new WebSocket("ws://localhost:3000/ws/live");
wsRef.current = ws;

ws.onopen = () => {
console.log("Orchestrator WebSocket connected successfully!");
setData(prev => ({
...prev,
stats: { ...prev.stats, orchestrator_online: true, ollama_online: true }
}));
setStats((p) => ({ ...p, orchestrator_online: true, ollama_online: true }));
};

ws.onmessage = (event) => {
try {
const raw = JSON.parse(event.data);
const { event: wsEvent, payload } = raw;
console.log("WS Telemetry Event:", wsEvent, payload);
const { event: wsEvent, payload } = JSON.parse(event.data);

if (wsEvent === "investigation_started") {
const pkgName = (payload.prompt || "unknown").replace("CLI Analysis: ", "");
setData(prev => ({
...prev,
package_name: pkgName,
const pkg = (payload.prompt || "unknown").replace("CLI Analysis: ", "");
setScanData((p) => ({
...p,
package_name: pkg,
status: "scanning",
stage: "sandbox",
confidence: 0,
total_severity: 0.0,
total_severity: 0,
events: [],
temporal_patterns: [],
network_connections: [],
ollama_triage: "AI Engine is analyzing the package behavioral trace..."
ollama_triage: "AI Engine analyzing...",
}));
} else if (wsEvent === "step_started") {
if (payload.stepId === "sandbox") setScanData((p) => ({ ...p, stage: "sandbox", confidence: 20 }));
else if (payload.stepId === "analysis") setScanData((p) => ({ ...p, stage: "randomforest", confidence: 60 }));
} else if (wsEvent === "step_completed" && payload.stepId === "analysis" && payload.findings) {
let findings = payload.findings;
if (typeof findings === "string") { try { findings = JSON.parse(findings); } catch { findings = {}; } }

const mappedEvents: SyscallEvent[] = (findings.events || []).map((e: any, idx: number) => ({
id: idx + 1,
syscall: e.syscall || "openat",
pid: e.pid || 0,
target: e.target || e.target_path || "unspecified",
severity: e.severity || "LOW",
flag: e.flag || false,
}));
}

else if (wsEvent === "step_started") {
if (payload.stepId === "sandbox") {
setData(prev => ({ ...prev, stage: "sandbox", confidence: 20 }));
} else if (payload.stepId === "analysis") {
setData(prev => ({ ...prev, stage: "randomforest", confidence: 60 }));
}
}

else if (wsEvent === "step_completed") {
if (payload.stepId === "sandbox" && payload.status === "completed") {
setData(prev => ({ ...prev, stage: "parser", confidence: 40 }));
} else if (payload.stepId === "analysis" && payload.findings) {
let findings = payload.findings;
if (typeof findings === "string") {
try {
findings = JSON.parse(findings);
} catch (e) {
findings = {};
}
}

// Map suspicious events
const mappedEvents = (findings.events || []).map((e: any, idx: number) => ({
id: idx + 1,
syscall: e.syscall || "openat",
pid: e.pid || 0,
target: e.target || e.target_path || "unspecified",
severity: e.severity || "LOW",
flag: e.flag || false
}));

// Map signatures
const mappedPatterns = (findings.behavioral_signatures || []).map((s: any) => ({
pattern: s.name || "suspicious_signature",
severity: s.severity === "HIGH" ? 9 : s.severity === "MEDIUM" ? 6 : 3,
window: s.evidence || "within execution window"
}));

// Map network
const mappedNetwork = (findings.network_destinations || []).map((n: any) => ({
destination: n.ip || n.host || "unknown",
port: n.port || 443,
classification: n.classification || "OUTBOUND",
verdict: n.is_malicious ? "MALICIOUS" : "CLEAN"
}));

setData(prev => ({
...prev,
status: findings.is_malicious ? "malicious" : "clean",
confidence: Math.round((findings.confidence_score || 0) * (findings.confidence_score <= 1 ? 100 : 1)),
stage: "ollama",
total_severity: findings.total_severity || 0.0,
events: mappedEvents,
temporal_patterns: mappedPatterns,
network_connections: mappedNetwork
}));
}
}
const mappedPatterns = (findings.behavioral_signatures || []).map((s: any) => ({
pattern: s.name || "suspicious_signature",
severity: s.severity === "HIGH" ? 9 : s.severity === "MEDIUM" ? 6 : 3,
window: s.evidence || "within execution window",
}));

else if (wsEvent === "ai_summary_started") {
setData(prev => ({
...prev,
ollama_triage: "Ollama is drafting final verification summary..."
const mappedNetwork = (findings.network_destinations || []).map((n: any) => ({
destination: n.ip || n.host || "unknown",
port: n.port || 443,
classification: n.classification || "OUTBOUND",
verdict: n.is_malicious ? "MALICIOUS" : "CLEAN",
}));
}

else if (wsEvent === "ai_summary_completed") {
setData(prev => ({
...prev,
stage: "complete",
ollama_triage: payload.summary || "AI Triage analysis complete."
const conf = Math.round((findings.confidence_score || 0) * (findings.confidence_score <= 1 ? 100 : 1));

setScanData((p) => ({
...p,
status: findings.is_malicious ? "malicious" : "clean",
confidence: conf,
stage: "ollama",
total_severity: findings.total_severity || 0,
events: mappedEvents,
temporal_patterns: mappedPatterns,
network_connections: mappedNetwork,
}));
// Refresh history
refreshScanHistory();
} else if (wsEvent === "ai_summary_completed") {
setScanData((p) => ({ ...p, stage: "complete", ollama_triage: payload.summary || "" }));
refreshHistory();
}

} catch (e) {
console.error("Error parsing WS event", e);
}
} catch { /* ignore bad ws frames */ }
};

ws.onclose = () => {
console.warn("Orchestrator WebSocket disconnected. Retrying in 3s...");
setData(prev => ({
...prev,
stats: { ...prev.stats, orchestrator_online: false, ollama_online: false }
}));
socketTimeout = setTimeout(connectWebSocket, 3000);
setStats((p) => ({ ...p, orchestrator_online: false, ollama_online: false }));
socketTimeout = setTimeout(connect, 3000);
};

ws.onerror = () => {
ws.close();
};
ws.onerror = () => ws.close();
};

connectWebSocket();
connect();
return () => {
clearTimeout(socketTimeout);
if (wsRef.current) wsRef.current.close();
wsRef.current?.close();
};
}, []);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Stale-closure bug: WS-triggered refreshHistory() keeps using the API key from mount time.

The WS effect has an empty dependency array (Line 242), so its ai_summary_completed handler (Line 222-224) permanently closes over the refreshHistory instance — and thus the apiKey — from the initial render. When the user later changes the API key (handleApiKeyChange), refreshHistory is redefined with the new key on re-render, but the long-lived WS handler never picks it up, so live-triggered history refreshes keep sending the stale key. LiveMonitor.tsx's equivalent WS effect correctly depends on [apiKey] and reconnects/rebinds on key change — this file diverges from that pattern.

🐛 Proposed fix: reconnect on apiKey change (mirrors LiveMonitor.tsx)
-  }, []);
+  }, [apiKey]);

Alternatively, keep the effect key-independent but read apiKey via a ref updated each render, so the long-lived closure always sees the latest value.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// WebSocket — mirrors old page.tsx logic
useEffect(() => {
let socketTimeout: ReturnType<typeof setTimeout>;
const connectWebSocket = () => {
console.log("Connecting to Orchestrator WebSocket...");
const connect = () => {
const ws = new WebSocket("ws://localhost:3000/ws/live");
wsRef.current = ws;
ws.onopen = () => {
console.log("Orchestrator WebSocket connected successfully!");
setData(prev => ({
...prev,
stats: { ...prev.stats, orchestrator_online: true, ollama_online: true }
}));
setStats((p) => ({ ...p, orchestrator_online: true, ollama_online: true }));
};
ws.onmessage = (event) => {
try {
const raw = JSON.parse(event.data);
const { event: wsEvent, payload } = raw;
console.log("WS Telemetry Event:", wsEvent, payload);
const { event: wsEvent, payload } = JSON.parse(event.data);
if (wsEvent === "investigation_started") {
const pkgName = (payload.prompt || "unknown").replace("CLI Analysis: ", "");
setData(prev => ({
...prev,
package_name: pkgName,
const pkg = (payload.prompt || "unknown").replace("CLI Analysis: ", "");
setScanData((p) => ({
...p,
package_name: pkg,
status: "scanning",
stage: "sandbox",
confidence: 0,
total_severity: 0.0,
total_severity: 0,
events: [],
temporal_patterns: [],
network_connections: [],
ollama_triage: "AI Engine is analyzing the package behavioral trace..."
ollama_triage: "AI Engine analyzing...",
}));
} else if (wsEvent === "step_started") {
if (payload.stepId === "sandbox") setScanData((p) => ({ ...p, stage: "sandbox", confidence: 20 }));
else if (payload.stepId === "analysis") setScanData((p) => ({ ...p, stage: "randomforest", confidence: 60 }));
} else if (wsEvent === "step_completed" && payload.stepId === "analysis" && payload.findings) {
let findings = payload.findings;
if (typeof findings === "string") { try { findings = JSON.parse(findings); } catch { findings = {}; } }
const mappedEvents: SyscallEvent[] = (findings.events || []).map((e: any, idx: number) => ({
id: idx + 1,
syscall: e.syscall || "openat",
pid: e.pid || 0,
target: e.target || e.target_path || "unspecified",
severity: e.severity || "LOW",
flag: e.flag || false,
}));
}
else if (wsEvent === "step_started") {
if (payload.stepId === "sandbox") {
setData(prev => ({ ...prev, stage: "sandbox", confidence: 20 }));
} else if (payload.stepId === "analysis") {
setData(prev => ({ ...prev, stage: "randomforest", confidence: 60 }));
}
}
else if (wsEvent === "step_completed") {
if (payload.stepId === "sandbox" && payload.status === "completed") {
setData(prev => ({ ...prev, stage: "parser", confidence: 40 }));
} else if (payload.stepId === "analysis" && payload.findings) {
let findings = payload.findings;
if (typeof findings === "string") {
try {
findings = JSON.parse(findings);
} catch (e) {
findings = {};
}
}
// Map suspicious events
const mappedEvents = (findings.events || []).map((e: any, idx: number) => ({
id: idx + 1,
syscall: e.syscall || "openat",
pid: e.pid || 0,
target: e.target || e.target_path || "unspecified",
severity: e.severity || "LOW",
flag: e.flag || false
}));
// Map signatures
const mappedPatterns = (findings.behavioral_signatures || []).map((s: any) => ({
pattern: s.name || "suspicious_signature",
severity: s.severity === "HIGH" ? 9 : s.severity === "MEDIUM" ? 6 : 3,
window: s.evidence || "within execution window"
}));
// Map network
const mappedNetwork = (findings.network_destinations || []).map((n: any) => ({
destination: n.ip || n.host || "unknown",
port: n.port || 443,
classification: n.classification || "OUTBOUND",
verdict: n.is_malicious ? "MALICIOUS" : "CLEAN"
}));
setData(prev => ({
...prev,
status: findings.is_malicious ? "malicious" : "clean",
confidence: Math.round((findings.confidence_score || 0) * (findings.confidence_score <= 1 ? 100 : 1)),
stage: "ollama",
total_severity: findings.total_severity || 0.0,
events: mappedEvents,
temporal_patterns: mappedPatterns,
network_connections: mappedNetwork
}));
}
}
const mappedPatterns = (findings.behavioral_signatures || []).map((s: any) => ({
pattern: s.name || "suspicious_signature",
severity: s.severity === "HIGH" ? 9 : s.severity === "MEDIUM" ? 6 : 3,
window: s.evidence || "within execution window",
}));
else if (wsEvent === "ai_summary_started") {
setData(prev => ({
...prev,
ollama_triage: "Ollama is drafting final verification summary..."
const mappedNetwork = (findings.network_destinations || []).map((n: any) => ({
destination: n.ip || n.host || "unknown",
port: n.port || 443,
classification: n.classification || "OUTBOUND",
verdict: n.is_malicious ? "MALICIOUS" : "CLEAN",
}));
}
else if (wsEvent === "ai_summary_completed") {
setData(prev => ({
...prev,
stage: "complete",
ollama_triage: payload.summary || "AI Triage analysis complete."
const conf = Math.round((findings.confidence_score || 0) * (findings.confidence_score <= 1 ? 100 : 1));
setScanData((p) => ({
...p,
status: findings.is_malicious ? "malicious" : "clean",
confidence: conf,
stage: "ollama",
total_severity: findings.total_severity || 0,
events: mappedEvents,
temporal_patterns: mappedPatterns,
network_connections: mappedNetwork,
}));
// Refresh history
refreshScanHistory();
} else if (wsEvent === "ai_summary_completed") {
setScanData((p) => ({ ...p, stage: "complete", ollama_triage: payload.summary || "" }));
refreshHistory();
}
} catch (e) {
console.error("Error parsing WS event", e);
}
} catch { /* ignore bad ws frames */ }
};
ws.onclose = () => {
console.warn("Orchestrator WebSocket disconnected. Retrying in 3s...");
setData(prev => ({
...prev,
stats: { ...prev.stats, orchestrator_online: false, ollama_online: false }
}));
socketTimeout = setTimeout(connectWebSocket, 3000);
setStats((p) => ({ ...p, orchestrator_online: false, ollama_online: false }));
socketTimeout = setTimeout(connect, 3000);
};
ws.onerror = () => {
ws.close();
};
ws.onerror = () => ws.close();
};
connectWebSocket();
connect();
return () => {
clearTimeout(socketTimeout);
if (wsRef.current) wsRef.current.close();
wsRef.current?.close();
};
}, []);
// WebSocket — mirrors old page.tsx logic
useEffect(() => {
let socketTimeout: ReturnType<typeof setTimeout>;
const connect = () => {
const ws = new WebSocket("ws://localhost:3000/ws/live");
wsRef.current = ws;
ws.onopen = () => {
setStats((p) => ({ ...p, orchestrator_online: true, ollama_online: true }));
};
ws.onmessage = (event) => {
try {
const { event: wsEvent, payload } = JSON.parse(event.data);
if (wsEvent === "investigation_started") {
const pkg = (payload.prompt || "unknown").replace("CLI Analysis: ", "");
setScanData((p) => ({
...p,
package_name: pkg,
status: "scanning",
stage: "sandbox",
confidence: 0,
total_severity: 0,
events: [],
temporal_patterns: [],
network_connections: [],
ollama_triage: "AI Engine analyzing...",
}));
} else if (wsEvent === "step_started") {
if (payload.stepId === "sandbox") setScanData((p) => ({ ...p, stage: "sandbox", confidence: 20 }));
else if (payload.stepId === "analysis") setScanData((p) => ({ ...p, stage: "randomforest", confidence: 60 }));
} else if (wsEvent === "step_completed" && payload.stepId === "analysis" && payload.findings) {
let findings = payload.findings;
if (typeof findings === "string") { try { findings = JSON.parse(findings); } catch { findings = {}; } }
const mappedEvents: SyscallEvent[] = (findings.events || []).map((e: any, idx: number) => ({
id: idx + 1,
syscall: e.syscall || "openat",
pid: e.pid || 0,
target: e.target || e.target_path || "unspecified",
severity: e.severity || "LOW",
flag: e.flag || false,
}));
const mappedPatterns = (findings.behavioral_signatures || []).map((s: any) => ({
pattern: s.name || "suspicious_signature",
severity: s.severity === "HIGH" ? 9 : s.severity === "MEDIUM" ? 6 : 3,
window: s.evidence || "within execution window",
}));
const mappedNetwork = (findings.network_destinations || []).map((n: any) => ({
destination: n.ip || n.host || "unknown",
port: n.port || 443,
classification: n.classification || "OUTBOUND",
verdict: n.is_malicious ? "MALICIOUS" : "CLEAN",
}));
const conf = Math.round((findings.confidence_score || 0) * (findings.confidence_score <= 1 ? 100 : 1));
setScanData((p) => ({
...p,
status: findings.is_malicious ? "malicious" : "clean",
confidence: conf,
stage: "ollama",
total_severity: findings.total_severity || 0,
events: mappedEvents,
temporal_patterns: mappedPatterns,
network_connections: mappedNetwork,
}));
} else if (wsEvent === "ai_summary_completed") {
setScanData((p) => ({ ...p, stage: "complete", ollama_triage: payload.summary || "" }));
refreshHistory();
}
} catch { /* ignore bad ws frames */ }
};
ws.onclose = () => {
setStats((p) => ({ ...p, orchestrator_online: false, ollama_online: false }));
socketTimeout = setTimeout(connect, 3000);
};
ws.onerror = () => ws.close();
};
connect();
return () => {
clearTimeout(socketTimeout);
wsRef.current?.close();
};
}, [apiKey]);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/app/page.tsx` around lines 151 - 242, Update the WebSocket useEffect
containing the ai_summary_completed handler so it does not retain the initial
refreshHistory/apiKey closure. Prefer matching LiveMonitor.tsx by adding apiKey
to the effect dependencies, allowing the socket handler to rebind when the key
changes; preserve the existing cleanup and reconnect behavior.

Comment on lines +64 to +76
const data = await res.json();
const vulns: CVEResult[] = (data.vulns || []).slice(0, 10).map((v: any) => {
const sev = v.severity?.[0];
const score = sev?.score ?? 0;
return {
id: v.id,
summary: v.summary || v.details?.slice(0, 120) || "",
severity: sev?.type || "UNKNOWN",
score,
url: `https://osv.dev/vulnerability/${v.id}`,
};
});
setCves(vulns);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- CommandConsole.tsx relevant slices ---'
nl -ba frontend/components/workbench/CommandConsole.tsx | sed -n '1,240p'

echo
echo '--- CVEResult definition and score consumers ---'
rg -n "interface CVEResult|type CVEResult|score\.toFixed|cvssToLevel|severity\\?\\.\\[0\\]\\.score|CVEResult" frontend -S

Repository: tejasprasad2008-afk/TraceTree

Length of output: 250


🏁 Script executed:

#!/bin/bash
set -euo pipefail
pwd
git ls-files frontend/components/workbench/CommandConsole.tsx

Repository: tejasprasad2008-afk/TraceTree

Length of output: 235


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- CommandConsole.tsx slices ---'
sed -n '1,240p' frontend/components/workbench/CommandConsole.tsx | cat -n

echo
echo '--- CVEResult definition and score consumers ---'
rg -n "interface CVEResult|type CVEResult|score\.toFixed|cvssToLevel|severity\\?\\.\\[0\\]\\.score|CVEResult" frontend -S

Repository: tejasprasad2008-afk/TraceTree

Length of output: 10936


🌐 Web query:

OSV API severity score CVSS vector string documentation

💡 Result:

In the Open Source Vulnerability (OSV) schema, severity information is represented using a JSON array named severity [1][2]. This array contains objects that define the severity of a vulnerability using quantitative scoring methods, typically CVSS vector strings [1][2]. The severity object contains the following properties: type: A string specifying the quantitative method used [1][2]. Valid types include: CVSS_V2: For CVSS version 2.0 (e.g., "AV:L/AC:M/Au:N/C:N/I:P/A:C") [1][2]. CVSS_V3: For CVSS versions 3.0 or 3.1 (e.g., "CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:C/C:H/I:N/A:N") [1][2]. CVSS_V4: For CVSS version 4.0 (e.g., "CVSS:4.0/AV:N/AC:L/AT:N/PR:H/UI:N/VC:L/VI:L/VA:N/SC:N/SI:N/SA:N") [1][2]. score: A string representing the actual CVSS vector string associated with the selected type [1][2]. source: An optional string indicating the origin of the severity assessment [2]. If omitted, the rating is implicitly attributed to the database that published the record [2]. The OSV API does not provide calculated numerical scores (e.g., "9.8") directly in its response; instead, it provides the CVSS vector strings [3]. Consumers requiring numerical scores must typically calculate them from the provided vector strings using external libraries or tools [3]. The severity field can be defined at the top level of the OSV record, and it may also be defined within the affected array to provide package-specific severity overrides [2][4][5].

Citations:


🌐 Web query:

OSV vulnerability severity score field CVSS vector string

💡 Result:

In the Open Source Vulnerability (OSV) schema, vulnerability severity information is captured using the severity field, which is a JSON array of objects [1][2]. Each object within this array contains a type and a score [2]. The severity[].score property is a string that represents the quantitative severity score based on the chosen severity[].type [1][2]. When the type is a Common Vulnerability Scoring System (CVSS) format, the score field contains the corresponding CVSS vector string [1][2]. Supported CVSS types include: - CVSS_V2: Uses a CVSS vector string (version 2.0) [1][2]. - CVSS_V3: Uses a CVSS vector string (versions >= 3.0 and < 4.0, such as 3.1) [1][2]. - CVSS_V4: Uses a CVSS vector string (versions >= 4.0 and < 5.0) [1][2]. The schema additionally supports an optional source property within each severity object to specify the origin of the severity data [2]. The structure is defined both in the JSON schema and the canonical Protocol Buffer definition (vulnerability.proto) [1][3].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,220p' frontend/components/workbench/SeverityTag.tsx | cat -n

Repository: tejasprasad2008-afk/TraceTree

Length of output: 1603


Convert OSV severity vectors before assigning to CVEResult.score. severity[].score is a CVSS vector string, so both mappers are putting a string into a number field; render paths then hit cve.score.toFixed(1) and cvssToLevel(...) with the wrong type.

  • frontend/components/workbench/CommandConsole.tsx#L64-L76
  • frontend/components/workbench/CommandConsole.tsx#L174-L183
📍 Affects 1 file
  • frontend/components/workbench/CommandConsole.tsx#L64-L76 (this comment)
  • frontend/components/workbench/CommandConsole.tsx#L174-L183
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/components/workbench/CommandConsole.tsx` around lines 64 - 76,
Convert each OSV severity vector to a numeric CVSS score before assigning
CVEResult.score, using the existing CVSS parsing/conversion utility. Apply this
in both mapper sites at frontend/components/workbench/CommandConsole.tsx lines
64-76 and 174-183, while preserving the existing fallback behavior when severity
data is missing.

? [...rows].sort((a, b) => {
const av = a[sortKey];
const bv = b[sortKey];
const cmp = String(av ?? "").localeCompare(String(bv ?? ""));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Fix numeric sorting for table columns.

The default localeCompare performs an alphabetical sort, which will incorrectly sort numerical values like confidence scores or IDs (e.g., "100" will sort before "92"). Supplying the { numeric: true } option handles both text and numbers seamlessly.

💡 Proposed fix
-        const cmp = String(av ?? "").localeCompare(String(bv ?? ""));
+        const cmp = String(av ?? "").localeCompare(String(bv ?? ""), undefined, { numeric: true });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const cmp = String(av ?? "").localeCompare(String(bv ?? ""));
const cmp = String(av ?? "").localeCompare(String(bv ?? ""), undefined, { numeric: true });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/components/workbench/DataTable.tsx` at line 55, Update the
comparison in the table sorting logic to pass the numeric option to
String.localeCompare, ensuring numeric-looking values such as IDs and confidence
scores sort numerically while preserving text sorting behavior.

Comment thread monitor/parser.py
Comment on lines 539 to 568
elif syscall in ("mmap", "mprotect"):
# mprotect with PROT_EXEC on RW memory is a red flag
has_prot_exec = "PROT_EXEC" in args_raw
is_rwx = False
if has_prot_exec:
# Low base severity — shared library loading does this constantly.
# The process_injection signature captures PROT_EXEC + non-standard
# binary together, which is the real high-severity pattern.
severity = max(severity, 3.0)
syscalls_executed.append(_make_event(
syscall,
f"flags={args_raw.split(',')[0] if ',' in args_raw else args_raw[:80]}",
severity,
{"has_prot_exec": has_prot_exec},
))
# DYNAMIC LINKER BASELINE: Do NOT flag mprotect on library memory regions as code injection
# UNLESS it explicitly sets PROT_EXEC on a previously writable (PROT_WRITE) region.
# Non-RWX mprotect (PROT_READ|PROT_EXEC) is the dynamic linker marking .text
# segments executable — completely routine for every shared library load.
# Adding severity 1.0 per call accumulated to 27.5+ on a benign pip install
# (which loads libc, libm, libz, etc.) and falsely crossed the high-severity boost
# threshold of 15.0, adding +30 to risk_score.
if "PROT_WRITE" in args_raw:
is_rwx = True
severity = max(severity, 8.0)
if not is_root_wrapper:
suspicious_flags.append(
f"Suspicious memory protection (RWX) in PID {pid}"
)

if not is_root_wrapper:
syscalls_executed.append(_make_event(
syscall,
f"flags={args_raw.split(',')[0] if ',' in args_raw else args_raw[:80]}",
severity,
{"has_prot_exec": is_rwx},
))
else:
severity = 0.0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

has_prot_exec detail key stores the wrong value and target string captures the wrong argument — both break the PROT_EXEC signature condition.

Two issues in this block:

  1. The emitted detail is {"has_prot_exec": is_rwx}, but is_rwx is only True when both PROT_EXEC and PROT_WRITE are present. monitor/signatures.py's _check_sequence_condition for "PROT_EXEC" reads details.get("has_prot_exec") expecting it to reflect plain PROT_EXEC presence per its own docstring ("PROT_EXEC" — mprotect with PROT_EXEC flag). Mislabeling it as is_rwx means mprotect/mmap calls with PROT_EXEC alone (no PROT_WRITE) will never satisfy this detail-based check.
  2. The fallback target string f"flags={args_raw.split(',')[0] if ',' in args_raw else args_raw[:80]}" takes the first comma-separated token of args_raw, which for mmap/mprotect syscalls is the memory address argument, not the protection flags. So the _check_sequence_condition fallback return "PROT_EXEC" in target (in monitor/signatures.py) will essentially never match either, since target never actually contains "PROT_EXEC".

Combined, this means the "PROT_EXEC" signature condition can only ever fire for RWX (PROT_EXEC+PROT_WRITE) mappings, silently missing exec-only mprotect/mmap calls — a real detection gap for a security-relevant syscall.

🐛 Suggested fix
-            if not is_root_wrapper:
-                syscalls_executed.append(_make_event(
-                    syscall,
-                    f"flags={args_raw.split(',')[0] if ',' in args_raw else args_raw[:80]}",
-                    severity,
-                    {"has_prot_exec": is_rwx},
-                ))
+            if not is_root_wrapper:
+                prot_match = re.search(r'PROT_[A-Z|]+', args_raw)
+                syscalls_executed.append(_make_event(
+                    syscall,
+                    f"flags={prot_match.group(0) if prot_match else args_raw[:80]}",
+                    severity,
+                    {"has_prot_exec": has_prot_exec, "is_rwx": is_rwx},
+                ))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
elif syscall in ("mmap", "mprotect"):
# mprotect with PROT_EXEC on RW memory is a red flag
has_prot_exec = "PROT_EXEC" in args_raw
is_rwx = False
if has_prot_exec:
# Low base severity — shared library loading does this constantly.
# The process_injection signature captures PROT_EXEC + non-standard
# binary together, which is the real high-severity pattern.
severity = max(severity, 3.0)
syscalls_executed.append(_make_event(
syscall,
f"flags={args_raw.split(',')[0] if ',' in args_raw else args_raw[:80]}",
severity,
{"has_prot_exec": has_prot_exec},
))
# DYNAMIC LINKER BASELINE: Do NOT flag mprotect on library memory regions as code injection
# UNLESS it explicitly sets PROT_EXEC on a previously writable (PROT_WRITE) region.
# Non-RWX mprotect (PROT_READ|PROT_EXEC) is the dynamic linker marking .text
# segments executable — completely routine for every shared library load.
# Adding severity 1.0 per call accumulated to 27.5+ on a benign pip install
# (which loads libc, libm, libz, etc.) and falsely crossed the high-severity boost
# threshold of 15.0, adding +30 to risk_score.
if "PROT_WRITE" in args_raw:
is_rwx = True
severity = max(severity, 8.0)
if not is_root_wrapper:
suspicious_flags.append(
f"Suspicious memory protection (RWX) in PID {pid}"
)
if not is_root_wrapper:
syscalls_executed.append(_make_event(
syscall,
f"flags={args_raw.split(',')[0] if ',' in args_raw else args_raw[:80]}",
severity,
{"has_prot_exec": is_rwx},
))
else:
severity = 0.0
elif syscall in ("mmap", "mprotect"):
# mprotect with PROT_EXEC on RW memory is a red flag
has_prot_exec = "PROT_EXEC" in args_raw
is_rwx = False
if has_prot_exec:
# DYNAMIC LINKER BASELINE: Do NOT flag mprotect on library memory regions as code injection
# UNLESS it explicitly sets PROT_EXEC on a previously writable (PROT_WRITE) region.
# Non-RWX mprotect (PROT_READ|PROT_EXEC) is the dynamic linker marking .text
# segments executable — completely routine for every shared library load.
# Adding severity 1.0 per call accumulated to 27.5+ on a benign pip install
# (which loads libc, libm, libz, etc.) and falsely crossed the high-severity boost
# threshold of 15.0, adding +30 to risk_score.
if "PROT_WRITE" in args_raw:
is_rwx = True
severity = max(severity, 8.0)
if not is_root_wrapper:
suspicious_flags.append(
f"Suspicious memory protection (RWX) in PID {pid}"
)
if not is_root_wrapper:
prot_match = re.search(r'PROT_[A-Z|]+', args_raw)
syscalls_executed.append(_make_event(
syscall,
f"flags={prot_match.group(0) if prot_match else args_raw[:80]}",
severity,
{"has_prot_exec": has_prot_exec, "is_rwx": is_rwx},
))
else:
severity = 0.0
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@monitor/parser.py` around lines 539 - 568, Update the mmap/mprotect event
construction in the syscall parsing block so details["has_prot_exec"] stores
has_prot_exec rather than is_rwx. Also build the event target/flags string from
the protection-flags argument that contains PROT_EXEC, instead of the first
address token, so the _check_sequence_condition fallback can detect it. Preserve
the existing RWX severity and suspicious-flag behavior.

Comment thread monitor/parser.py
Comment on lines 742 to +759
else:
syscalls_executed.append(_make_event(
syscall,
"other",
0.0,
{},
))
if not is_root_wrapper:
syscalls_executed.append(_make_event(
syscall,
"other",
0.0,
{},
))

# Filter out remaining events from root_wrapper so they don't pollute logs
if is_root_wrapper and syscall not in ("execve", "mmap", "mprotect", "clone", "fork", "vfork"):
# Already handled some above, for others we just pop the last appended event if it was appended
# Actually, to be safer, we can just intercept them in each branch.
# But the simplest is to only add severity if not root_wrapper.
severity = 0.0
# Remove from syscalls_executed if it was just added
if syscalls_executed and syscalls_executed[-1]["pid"] == pid and syscalls_executed[-1]["sequence_id"] == sequence_id:
syscalls_executed.pop()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift

Root-wrapper event cleanup can pop an unrelated, legitimately-kept event when no event was appended this iteration.

The pop-guard at lines 757-759 relies on syscalls_executed[-1]["sequence_id"] == sequence_id, but sequence_id is a module-level counter only incremented inside _make_event. If the current root-wrapper syscall doesn't reach a branch that calls _make_event this iteration (e.g. a malformed connect/file-access line where the regex doesn't match, or any syscall hitting the "other" branch with is_root_wrapper — no new event pushed), sequence_id still equals whatever the last successfully created event's id was. If that last event's pid also happens to equal root_pid — which is true for legitimately-kept root-wrapper clone/fork/vfork events (exempted from this cleanup at their own iteration) — this check spuriously matches and pops that legitimate prior event, corrupting parent_map/process-tree data that build_cascade_graph relies on.

Also note the separate, unrelated "chown" gap: chown isn't included in the file-access elif tuple at line 516, so it falls through to this "other" branch, where the event's recorded severity is hardcoded to 0.0 even though total_severity still accumulates the 0.5 weight from SEVERITY_WEIGHTS["chown"] — an event/total-severity mismatch.

🐛 Suggested fix for the stale sequence_id pop bug
+        _pre_branch_len = len(syscalls_executed)
+
         # ------------------------------------------------------------------ #
         #  Any other syscall — track it but don't flag
         # ------------------------------------------------------------------ #
         else:
             if not is_root_wrapper:
                 syscalls_executed.append(_make_event(
                     syscall,
                     "other",
-                    0.0,
+                    severity,
                     {},
                 ))

         # Filter out remaining events from root_wrapper so they don't pollute logs
-        if is_root_wrapper and syscall not in ("execve", "mmap", "mprotect", "clone", "fork", "vfork"):
+        if (is_root_wrapper
+                and syscall not in ("execve", "mmap", "mprotect", "clone", "fork", "vfork")
+                and len(syscalls_executed) > _pre_branch_len):
             severity = 0.0
-            if syscalls_executed and syscalls_executed[-1]["pid"] == pid and syscalls_executed[-1]["sequence_id"] == sequence_id:
-                syscalls_executed.pop()
+            syscalls_executed.pop()

Also add "chown" to the file-access tuple at line 516 so it gets proper sensitive-path evaluation instead of falling into the generic "other" bucket.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@monitor/parser.py` around lines 742 - 759, Fix root-wrapper cleanup in the
syscall event-processing flow so it removes an event only when the current
iteration actually appended one; do not rely on the module-level sequence_id,
which may refer to an older event. Preserve legitimate root-wrapper
clone/fork/vfork events. Also add chown to the file-access syscall tuple so it
uses sensitive-path evaluation and records severity consistently with
total_severity.

Comment thread monitor/yara.py
Comment on lines +156 to +178

/* TODO: Written 2026-07-15 to unblock LummaStealer PS1-dropper end-to-end
verification test. Needs a deliberate pass before relying on it as a
real detection rule: false-positive rate unvalidated (BITSTransfer is
used by legitimate Windows software), pattern coverage narrow
(only covers BITSTransfer+RunKey variant; omits mshta, wscript, regsvr32,
certutil, and other common PS1 dropper LOLBins). */
rule PowerShellDropper {
meta:
description = "Detects PowerShell download-execute-persist dropper pattern"
severity = "critical"
mitre = "T1059.001,T1547.001,T1197"
strings:
$bits = "BITSTransfer" nocase
$bits2 = "Start-BitsTransfer" nocase
$reg_run = "CurrentVersion\\Run" nocase
$appdata_exe = /APPDATA.*\.exe/ nocase
$expand_archive = "Expand-Archive" nocase
$start_process = "Start-Process" nocase
condition:
($bits or $bits2) and $reg_run
or ($appdata_exe and $expand_archive and $start_process)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Self-flagged unvalidated rule shipped into the production ruleset.

The TODO explicitly states the rule "Needs a deliberate pass before relying on it as a real detection rule: false-positive rate unvalidated (BITSTransfer is used by legitimate Windows software), pattern coverage narrow". As written, PowerShellDropper participates in scan_with_yara's normal match path and will surface as a "critical" severity finding (feeding into YaraAnalyzer's MALICIOUS verdict) despite the author's own acknowledgment that it's unvalidated.

Would you like this gated behind a lower initial severity (or excluded from the compiled ruleset) until the false-positive rate is validated? I can open a tracking issue if useful.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@monitor/yara.py` around lines 156 - 178, Remove PowerShellDropper from the
production-compiled YARA ruleset, or gate it behind the project’s existing
experimental/unvalidated-rule mechanism so scan_with_yara cannot report it
through the normal critical-severity path. Preserve the rule definition for
end-to-end verification, and ensure YaraAnalyzer does not classify its matches
as MALICIOUS until validation is complete.

Comment thread monitor/yara.py
Comment on lines +222 to 243
instances = [
instance
for string_match in m.strings
for instance in string_match.instances
]
results.append({
"rule_name": m.rule,
"severity": m.meta.get("severity", "unknown"),
"description": m.meta.get("description", ""),
"file_path": str(fpath),
"matched_strings": [
instance.matched_data.decode('utf-8', errors='replace')
for string_match in m.strings
for instance in string_match.instances
inst.matched_data.decode('utf-8', errors='replace')
for inst in instances
],
# Parallel field preserving byte offsets for static analysis.
# matched_strings kept as plain strings for backward compat.
"matched_offsets": [
{"text": inst.matched_data.decode('utf-8', errors='replace'),
"offset": inst.offset}
for inst in instances
],
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check pinned yara-python version across manifest files.
rg -n 'yara-python' pyproject.toml setup.py requirements*.txt 2>/dev/null

Repository: tejasprasad2008-afk/TraceTree

Length of output: 167


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== candidate files ==\n'
git ls-files | rg '(^|/)(pyproject\.toml|setup\.py|requirements.*\.txt|Pipfile|poetry\.lock|uv\.lock|monitor/yara\.py)$'

printf '\n== yara references ==\n'
rg -n 'yara|StringMatch|matched_data|fallback_regex_scan|scan_with_yara' .

printf '\n== monitor/yara.py outline ==\n'
ast-grep outline monitor/yara.py --view expanded

printf '\n== monitor/yara.py relevant section ==\n'
sed -n '1,320p' monitor/yara.py

Repository: tejasprasad2008-afk/TraceTree

Length of output: 27250


🏁 Script executed:

#!/bin/bash
set -euo pipefail
git ls-files | rg '(^|/)(pyproject\.toml|setup\.py|requirements.*\.txt|Pipfile|poetry\.lock|uv\.lock|monitor/yara\.py)$'
rg -n 'yara|StringMatch|matched_data|fallback_regex_scan|scan_with_yara' .
sed -n '1,320p' monitor/yara.py

Repository: tejasprasad2008-afk/TraceTree

Length of output: 26713


🏁 Script executed:

#!/bin/bash
set -euo pipefail
pwd
git ls-files | head -n 50

Repository: tejasprasad2008-afk/TraceTree

Length of output: 1696


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== pyproject excerpt ==\n'
rg -n -C 3 'yara|dependencies|install_requires|packages' pyproject.toml setup.py requirements-ingest.txt

printf '\n== full dependency blocks ==\n'
python3 - <<'PY'
from pathlib import Path
for path in ["pyproject.toml", "setup.py", "requirements-ingest.txt"]:
    p = Path(path)
    if p.exists():
        print(f"\n--- {path} ---")
        text = p.read_text(encoding="utf-8", errors="replace").splitlines()
        for i, line in enumerate(text, 1):
            if any(k in line.lower() for k in ("yara", "dependencies", "install_requires", "extras_require")):
                start = max(1, i-4)
                end = min(len(text), i+12)
                for j in range(start, end+1):
                    print(f"{j}:{text[j-1]}")
                print()
PY

printf '\n== scan_with_yara control flow ==\n'
sed -n '186,250p' monitor/yara.py

Repository: tejasprasad2008-afk/TraceTree

Length of output: 4965


Support both YARA APIs or pin yara-python to 4.3+
monitor/yara.py:222-243 assumes Match.strings yields StringMatch objects. Older yara-python releases return tuples here, so string_match.instances / inst.matched_data will fail and the scan can return no YARA findings for every file. Either pin the dependency or handle both result shapes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@monitor/yara.py` around lines 222 - 243, Update the YARA match processing
that builds instances and matched_offsets to support both modern StringMatch
objects and legacy tuple results from yara-python, or enforce yara-python 4.3+
through the project dependency configuration. Preserve matched_strings and
byte-offset output for supported API shapes, ensuring scans do not fail when
older results are returned.

Comment thread sandbox/sandbox.py
Comment on lines +652 to +653
except Exception as _qe:
log.warning("Quarantine extraction failed (non-fatal): %s", _qe)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -nP '^\s*(import logging|from logging|log\s*=|import\s+.*\bas\s+log\b)' sandbox/sandbox.py

Repository: tejasprasad2008-afk/TraceTree

Length of output: 167


log is undefined here — the quarantine fallback can crash instead of staying non-fatal.
sandbox/sandbox.py only uses console in this module, so log.warning(...) raises NameError inside the except block. Use the in-scope output path (console.print(...)) or a real logger.

🧰 Tools
🪛 Ruff (0.15.21)

[warning] 652-652: Do not catch blind exception: Exception

(BLE001)


[error] 653-653: Undefined name log

(F821)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sandbox/sandbox.py` around lines 652 - 653, Replace the undefined log.warning
call in the quarantine extraction exception handler with the module’s in-scope
console.print output path, preserving the error message and ensuring this
fallback remains non-fatal.

Source: Linters/SAST tools

@tejasprasad2008-afk
tejasprasad2008-afk marked this pull request as draft July 21, 2026 13:42

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
sandbox/sandbox.py (2)

665-671: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Bound quarantine archive size before writing and extracting it.

The archive comes from attacker-controlled ZIP contents, but this copies the entire tar to the host and extracts it without byte, file-count, or member-count limits. A decompression bomb or archive containing millions of files can exhaust host disk or memory before the non-fatal handler runs. Enforce bounded streaming and extraction limits.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sandbox/sandbox.py` around lines 665 - 671, Bound the quarantine archive
handling around _extract_tar_safely: while streaming q_stream into q_tar_tmp,
track bytes and abort once a defined maximum archive size is exceeded, and
enforce limits on extracted file count and total extracted bytes (including tar
members) before writing files. Ensure temporary resources are cleaned up and the
existing non-fatal error path handles limit violations.

665-672: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Clean up the temporary quarantine tar on failure.

q_tar_tmp is removed only after successful parsing and extraction. Exceptions leave temporary archives in the system temp directory, allowing repeated malformed scans to accumulate disk usage. Remove the file in a finally block.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sandbox/sandbox.py` around lines 665 - 672, Wrap the temporary quarantine tar
download, parsing, and extraction flow around q_tar_tmp in a finally block, and
remove q_tar_tmp.name there so cleanup occurs on both success and failure.
Preserve the existing archive processing behavior while ensuring cleanup also
handles exceptions from get_archive, tarfile.open, or _extract_tar_safely.
cli.py (1)

308-313: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Guard TRACETREE_QUARANTINE_TTL_DAYS against non-numeric values.

int(os.environ.get("TRACETREE_QUARANTINE_TTL_DAYS", max_age_days)) raises ValueError when the env var is set to a non-integer. Since _purge_stale_quarantine() runs at the top of perform_analysis, a malformed value crashes every analyze invocation rather than degrading gracefully.

🛡️ Proposed guard
-    ttl = int(os.environ.get("TRACETREE_QUARANTINE_TTL_DAYS", max_age_days))
+    try:
+        ttl = int(os.environ.get("TRACETREE_QUARANTINE_TTL_DAYS", max_age_days))
+    except (TypeError, ValueError):
+        ttl = max_age_days
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cli.py` around lines 308 - 313, Update the TTL parsing in
_purge_stale_quarantine so a non-numeric TRACETREE_QUARANTINE_TTL_DAYS value
does not raise ValueError or abort perform_analysis. Catch the conversion
failure and fall back to max_age_days, preserving the existing cutoff and
quarantine cleanup behavior for valid values.
🧹 Nitpick comments (1)
orchestrator/src/llm/index.ts (1)

234-240: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Normalize the provider name configuration to lowercase.

Users might mistakenly supply uppercase or mixed-case values for the LLM_PROVIDER environment variable (e.g., OpenAI instead of openai). Using .toLowerCase() will make the configuration more resilient and prevent fallback to the default mock provider or throwing an unsupported provider error.

💡 Proposed refactor
-  const provider = process.env.LLM_PROVIDER || 'mock';
+  const provider = (process.env.LLM_PROVIDER || 'mock').toLowerCase();
   
   logger.info('llm', `Initializing provider: ${provider}`);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@orchestrator/src/llm/index.ts` around lines 234 - 240, Normalize the
LLM_PROVIDER value to lowercase when assigning provider, while preserving the
existing mock fallback when the environment variable is unset. Keep the
logger.info initialization flow unchanged so mixed-case provider names are
passed consistently to downstream provider selection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@api/main.py`:
- Around line 120-133: Update _update_env_file to restrict the persisted .env
file to owner-only permissions after path.write_text completes, applying the
mode for both newly created and existing files without changing the current
content-update behavior.

In `@frontend/components/workbench/FirstRunSetup.tsx`:
- Line 64: Update the displayStep calculation in FirstRunSetup to handle step
=== 1 by mapping it to visual progress step 2, while preserving the existing
mappings for steps 0, 2, and 3.

---

Outside diff comments:
In `@cli.py`:
- Around line 308-313: Update the TTL parsing in _purge_stale_quarantine so a
non-numeric TRACETREE_QUARANTINE_TTL_DAYS value does not raise ValueError or
abort perform_analysis. Catch the conversion failure and fall back to
max_age_days, preserving the existing cutoff and quarantine cleanup behavior for
valid values.

In `@sandbox/sandbox.py`:
- Around line 665-671: Bound the quarantine archive handling around
_extract_tar_safely: while streaming q_stream into q_tar_tmp, track bytes and
abort once a defined maximum archive size is exceeded, and enforce limits on
extracted file count and total extracted bytes (including tar members) before
writing files. Ensure temporary resources are cleaned up and the existing
non-fatal error path handles limit violations.
- Around line 665-672: Wrap the temporary quarantine tar download, parsing, and
extraction flow around q_tar_tmp in a finally block, and remove q_tar_tmp.name
there so cleanup occurs on both success and failure. Preserve the existing
archive processing behavior while ensuring cleanup also handles exceptions from
get_archive, tarfile.open, or _extract_tar_safely.

---

Nitpick comments:
In `@orchestrator/src/llm/index.ts`:
- Around line 234-240: Normalize the LLM_PROVIDER value to lowercase when
assigning provider, while preserving the existing mock fallback when the
environment variable is unset. Keep the logger.info initialization flow
unchanged so mixed-case provider names are passed consistently to downstream
provider selection.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2c034779-9a20-4cea-a32e-45c9df3d8397

📥 Commits

Reviewing files that changed from the base of the PR and between 072e1d4 and 84a28f2.

📒 Files selected for processing (13)
  • .env.example
  • .gitignore
  • README.md
  • api/main.py
  • cli.py
  • frontend/app/page.tsx
  • frontend/components/workbench/FirstRunSetup.tsx
  • orchestrator/ai_mesh.py
  • orchestrator/src/llm/index.test.ts
  • orchestrator/src/llm/index.ts
  • orchestrator/src/server.ts
  • orchestrator/src/store/index.ts
  • sandbox/sandbox.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • .gitignore
  • frontend/app/page.tsx

Comment thread api/main.py
Comment on lines +120 to +133
def _update_env_file(path: Path, values: Dict[str, str]) -> None:
"""Update selected values without echoing secrets or replacing unrelated config."""
existing = path.read_text() if path.exists() else ""
lines = existing.splitlines()
pending = dict(values)
output: List[str] = []
for line in lines:
key = line.split("=", 1)[0].strip() if "=" in line else ""
if key in pending:
output.append(f"{key}={pending.pop(key)}")
else:
output.append(line)
output.extend(f"{key}={value}" for key, value in pending.items())
path.write_text("\n".join(output).rstrip() + "\n")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Restrict .env permissions after writing provider secrets.

_update_env_file persists API keys (e.g. OPENAI_API_KEY) via write_text, which on first creation leaves default (typically world-readable 0644) permissions. Since this file holds cloud credentials, tighten it to owner-only after writing.

🔒 Proposed hardening
     output.extend(f"{key}={value}" for key, value in pending.items())
     path.write_text("\n".join(output).rstrip() + "\n")
+    try:
+        path.chmod(0o600)
+    except OSError:
+        pass
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def _update_env_file(path: Path, values: Dict[str, str]) -> None:
"""Update selected values without echoing secrets or replacing unrelated config."""
existing = path.read_text() if path.exists() else ""
lines = existing.splitlines()
pending = dict(values)
output: List[str] = []
for line in lines:
key = line.split("=", 1)[0].strip() if "=" in line else ""
if key in pending:
output.append(f"{key}={pending.pop(key)}")
else:
output.append(line)
output.extend(f"{key}={value}" for key, value in pending.items())
path.write_text("\n".join(output).rstrip() + "\n")
def _update_env_file(path: Path, values: Dict[str, str]) -> None:
"""Update selected values without echoing secrets or replacing unrelated config."""
existing = path.read_text() if path.exists() else ""
lines = existing.splitlines()
pending = dict(values)
output: List[str] = []
for line in lines:
key = line.split("=", 1)[0].strip() if "=" in line else ""
if key in pending:
output.append(f"{key}={pending.pop(key)}")
else:
output.append(line)
output.extend(f"{key}={value}" for key, value in pending.items())
path.write_text("\n".join(output).rstrip() + "\n")
try:
path.chmod(0o600)
except OSError:
pass
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@api/main.py` around lines 120 - 133, Update _update_env_file to restrict the
persisted .env file to owner-only permissions after path.write_text completes,
applying the mode for both newly created and existing files without changing the
current content-update behavior.

const selected = PROVIDERS.find((item) => item.id === provider)!;
const apiBase = process.env.NEXT_PUBLIC_TRACETREE_API_URL || "http://127.0.0.1:8000";
const totalSteps = selected.requiresApiKey ? 4 : 3;
const displayStep = step === 0 ? 1 : step === 2 ? (selected.requiresApiKey ? 3 : 2) : step === 3 ? totalSteps : 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Fix progress step calculation for the API key step.

When a provider requires an API key, the setup flows to step === 1. The current ternary logic does not handle step === 1, causing displayStep to fall back to 0. This results in the UI displaying "STEP 0 / 4" and coloring zero segments in the progress bar on this screen.

Add a condition for step === 1 to correctly map it to step 2 visually.

🐛 Proposed fix
-  const displayStep = step === 0 ? 1 : step === 2 ? (selected.requiresApiKey ? 3 : 2) : step === 3 ? totalSteps : 0;
+  const displayStep = step === 0 ? 1 : step === 1 ? 2 : step === 2 ? (selected.requiresApiKey ? 3 : 2) : step === 3 ? totalSteps : 0;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const displayStep = step === 0 ? 1 : step === 2 ? (selected.requiresApiKey ? 3 : 2) : step === 3 ? totalSteps : 0;
const displayStep = step === 0 ? 1 : step === 1 ? 2 : step === 2 ? (selected.requiresApiKey ? 3 : 2) : step === 3 ? totalSteps : 0;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/components/workbench/FirstRunSetup.tsx` at line 64, Update the
displayStep calculation in FirstRunSetup to handle step === 1 by mapping it to
visual progress step 2, while preserving the existing mappings for steps 0, 2,
and 3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant