Repository navigation
feat: add vt-tool-ui frontend (Analyze, History, MISP push) - #14
Merged
Merged
Conversation
Sets up an isolated venv + tests/ package so python -m unittest discover -s tests -t . works, and records the full implementation plan this branch executes task-by-task. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Task 7's implementer found that create_schema()'s SCHEMA uses AUTOINCREMENT, which makes SQLite auto-create an internal sqlite_sequence bookkeeping table alongside the 4 real tables. The plan's test_create_schema_creates_all_tables assertion didn't account for it (an oversight from planning verification, not implementer error) — filter it out before comparing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
app/DataHandler/validator.py's TLDExtract cache warms a .lock file under app/DataHandler/public/ on first use; it's a transient runtime artifact of running the test suite, not something to track. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Task 10's review found the plan's own test didn't actually exercise re.IGNORECASE (all sample filenames already matched in the pattern's native case). Keeps the plan doc consistent with the fix applied to tests/test_misp.py. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ng, stdin coverage)
The final-review fix cycle discovered a third real bug (ValueReader defines _accumulate_values twice, silently breaking stdin-piped input) but the plan's closing summary still only listed the two DBHandler bugs and the extract_table_data fragility. Add it so the audit-phase findings list is complete. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Completes the earlier request to keep .claude/ and CLAUDE.md out of git, carried over after merging the unit-test-suite branch (which added its own .venv/ and *.tldextract.json.lock entries to the same file). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ValueReader defined _accumulate_values twice: once for the stdin
path (writes to self.dict_values) and again for the file path
(writes to self.dict_values_file). Python keeps only the second
definition in the class namespace, so read_from_stdin() called the
file-path version, wrote into the wrong dict, and always returned
{} while printing "Successfully read values from user input" -
silently breaking the CLI's stdin-piping input mode.
Renamed the file-path method to _accumulate_file_values so the two
no longer collide. tests/test_read_file.py's
test_read_from_stdin_parses_piped_lines now asserts the correct
behavior instead of characterizing the bug.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two bugs in the local-cache read path: 1. exists() compared stored values against the literal "Not Found" (capital F), but NOT_FOUND_ERROR (what's actually stored) is "Not found" (lowercase f). The mismatch meant the ratio check meant to treat mostly-empty cached rows as cache misses never fired - a value that came back empty from VT once would be served from that useless row forever. Fixed by comparing against NOT_FOUND_ERROR instead of the hardcoded literal. 2. populate_scores()/populate_tags() read malicious_score/ total_scans/tags from fixed tuple offsets (report[2]/[3]/[4]) that only match the hashes table's column layout. For ips and domains, those offsets land on unrelated columns (ip/port/ protocol), so cached IP/domain lookups showed corrupted values for those three fields. Fixed by adding a SCORE_TAG_COLUMNS map of column positions per value_type and having both methods look up the right indices instead of assuming one fixed layout. tests/test_db_handler.py's two characterization tests for these bugs now assert the correct behavior instead of the old (buggy) one, and a new test_ip_report_score_and_tag_fields_are_correct covers the ips table specifically (previously only domains had a dedicated round-trip regression test for this). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
headers was accumulated as a set in the same loop that built rows, so each row was built from headers as it stood after that single result, not the final combined set. Results with heterogeneous keys produced rows shorter than the returned header list, silently misaligning columns (masked in practice by CustomPrettyTable dropping any row whose length doesn't match). Split into two passes: accumulate the complete header set over all results first, then build every row against that final set. test_rows_are_fully_populated_with_final_headers replaces the old characterization test and asserts every row's length matches the header count, with values verified to align to the correct header by name (not just row length). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Findings-only report (nothing applied yet): ponytail-audit pass over app/, vt_tools.py, init.py plus a dependency and library-replacement research pass. Highlights: 8 of 15 requirements.txt entries are unused dead weight, an entire unused pydantic validation block, duplicated VTReporter/DBHandler report-shaping logic, and two IOC-extraction library candidates (iocextract, ioc-finder) as possible replacements for the hand-rolled regex in read_file.py. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- validator.py: drop the entire unused Pydantic validation block (IPModel/DomainModel/HashModel/URLModel) - never used outside this file, duplicates what validate_ip/validate_domain/validate_hash/ validate_url already do. - validator.py: drop self.hashid = HashID() (never read - the real validate_hash path classifies by length/regex, not via hashid) and the empty_ssdeep dead-end branch (already falls through to the same `return None`). - validator.py: drop unused `hashid`/`pydantic` imports, plus `hashlib` (never referenced at all). - validator.py: collapse the unreachable `except OSError` clause in get_service_name/get_port_from_service_name - socket.error has been an alias for OSError since Python 3.3, so it could never fire after the preceding `except (socket.error, ValueError)`. - vt_reporter.py: drop populate_threat_classification - defined, never called (the same logic is already inlined in populate_value_object). - utils.py: drop the unused `from pytz import timezone as pytz_timezone` import. 113/113 tests still pass, no test changes needed - nothing in the suite exercised any of this dead code. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Removed 8 unused entries: click (vt_tools.py uses argparse, not click), setuptools (no direct import), ipaddress (stdlib since Python 3.3, not a pip package to install), urllib3 (no direct import - already a transitive dep of requests/vt-py), tenacity (no references), pytz (no references, its only import was removed in the previous commit), hashid and pydantic (both only reachable from dead code removed in the previous commit). Verified by building a completely fresh venv from the trimmed file (not just relying on the existing dev venv still having the old packages installed) and running the full 113-test suite plus a `vt_tools.py --help` smoke test against it - both clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Findings 1-5, 9-10 and the full requirements.txt dependency cleanup are applied (see d165569, 6de0f74). Findings 6-8 (the three shrink: duplications) and the two structural items are still open for the next tier. Also notes a tldextract deprecation warning (registered_domain -> top_domain_under_public_suffix) surfaced during fresh-venv verification, flagged for later. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…late_link VTReporter and DBHandler each had a byte-for-byte identical populate_link method (build a VT GUI URL from a value/value_type). Extracted to app.DataHandler.utils.build_virustotal_link - a leaf module both already depend on transitively (VTReporter already imports from it; DBHandler importing from it doesn't risk a cycle since utils.py doesn't import DBHandler or VirusTotal). DBHandler's `from vt import url_id` import is now unused and removed - the only thing that used it was the deleted method. Added tests/test_utils.py::BuildVirustotalLinkTests - the link computation had no direct test coverage before (only exercised implicitly through create_object/get_report, which don't assert on the link field), so this is new coverage, not just relocated. 116/116 tests pass (113 + 3 new). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The two functions were structurally identical - same prompt text, same branching, same recursive retry-on-invalid-input - differing only in whether template_file/template were passed through to misp_event. Merged into one misp_choice(case_str, csvfilescreated, template_file=None, template=None); the non-template call site is unaffected (defaults preserve the old misp_event(..., None, None) behavior), the template call site now calls misp_choice directly. Added tests/test_misp.py::MispChoiceTests - this had zero test coverage before (out of scope in the original test-writing plan since it seemed to need a live/mocked MISP server), but the actual MISP-server call is fully contained in the separate misp_event function, so mocking just that one call makes the prompt-handling logic (yes/no/invalid-retry/template passthrough/000000 event-ID prompt) straightforward to test without touching MISP at all. 121/121 tests pass (116 + 5 new). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The same 9-entry list of validate_value() return strings that shouldn't be queried/fetched (private/reserved IP ranges, hash types VT doesn't accept) was duplicated verbatim in both get_existing_report and analyze_value. Extracted to a module-level UNSUPPORTED_VALUE_TYPES constant (a set, since it's only ever used for membership tests) that both now reference. Added a test pinning the exact expected membership, so a future edit that accidentally drops or duplicates an entry gets caught. 122/122 tests pass (121 + 1 new). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Also fixes vite.config.ts: vitest's default include and Playwright's e2e spec both match *.spec.ts, so vitest tried to execute Playwright's test() under its own runner. Excludes e2e/** from vitest's test run (Playwright's own runner still exercises e2e/analyze.spec.ts). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Consolidated fix wave for the 8 cross-cutting findings from the vt-tool-ui whole-branch final review (error handling, caching, docs, tests): - AnalyzePage: preserve the reviewed batch and surface an error banner on a failed /analyze submit instead of destroying the user's input; add a "New analysis" reset button; surface job-polling transport errors instead of polling a dead endpoint forever. - nginx.conf: no-cache env-config.js instead of caching it immutably for a year alongside hashed static assets. - vt-tool-ui/.dockerignore: keep host node_modules/dist out of the Docker build context (plus a narrow .gitignore negation so the file actually ships to other clones). - Add AnalyzePage test coverage for the queued -> poll -> render path (previously untested end to end) and for the submit-failure path. - computeVerdict: treat total_scans === 0 as unknown, matching the real backend's not-found shape, instead of rendering it as CLEAN. - Document vt-tool-ui in the root README, deployment/README.md, and a new vt-tool-ui/README.md. - ci-smoke.sh: scope the compose build to backend services only, so an unrelated frontend build failure can't fail the backend smoke test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Six small items deferred from frontend-v1's final review, none of them blocking: ResultsTable now keys rows by index so duplicate IOCs don't collide; AnalyzePage shows a warning banner linking to Settings when no API key is stored instead of silently submitting an empty one; the vt-tool-ui Compose service gets a Docker healthcheck against its own /healthz; vite.config.ts/playwright.config.ts/e2e are now covered by tsc via a new tsconfig.node.json; IocInput's drop path (5MB cap rejection, appended content) is now tested and file.text() has a .catch; ApiHealthIndicator shows a neutral "checking" state instead of flashing red while the first health check is in flight. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
HistoryService uses its own raw sqlite3 connection rather than the cache's pluggable SQLAlchemy backend (VT_CACHE_DB_URL). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Promote ATTRIBUTE_TYPE_MAPPING and OBJECT_NAME_BY_VALUE_TYPE to module-level constants in app/services/misp_service.py. Previously, ATTRIBUTE_TYPE_MAPPING was defined locally in process_and_submit_to_misp() with no reuse; now it lives in a shared location so the API's MISP push endpoint (Task 4) can import both constants and map value types to MISP objects without going through CSV. - Added OBJECT_NAME_BY_VALUE_TYPE to map (ips/domains/urls/hashes) to MISP object names (ip-port/domain-ip/url/file) - Added ATTRIBUTE_TYPE_MAPPING with all object-type mappings plus "general" - Updated vt_tools2misp.py to import and use ATTRIBUTE_TYPE_MAPPING - Added three tests to verify constant structure and coverage - CLI behavior unchanged: pure data-source swap Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add three new history endpoints to app/api/main.py:
- POST /analyses: save an analysis with items and optional case_label
- GET /analyses: list saved analyses with limit/offset (max 100 per spec)
- GET /analyses/{analysis_id}: get full detail of one analysis, 404 if missing
Wires up HistoryService in FastAPI lifespan (setup before yield, cleanup
after). Adds AnalysisItem and SaveAnalysisRequest Pydantic models matching
the brief's exact shape for frontend consumption.
All 6 new tests pass, full suite (217 tests) passes, ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Consolidated fix wave for frontend-v2's final whole-branch review, covering
6 cross-cutting issues invisible to per-task review because every task
mocked the MISP interaction:
1. get_misp_event never detected pymisp's non-raising 404 (get_event
returns {'errors': ...} instead of raising), so MISP push 502'd on any
run without a pre-existing exact-match event - i.e. always, by default.
2. submit_misp_objects now returns the count of objects MISP actually
accepted (add_object not raising), and the push endpoint uses that for
pushed_count instead of the count merely attempted.
3. The push endpoint now flattens nested report fields (info-ip.asn,
info-ip.regional_internet_registry) into the top-level dict before
building MISP objects, instead of silently dropping them, and excludes
other dict-valued fields (info, https_certificate) that have no
per-subfield mapping.
4. client.ts gained a response interceptor that rewrites error.message to
the server's `detail` field, so MispPushControl's error branch shows the
real failure reason instead of axios's generic status-code message.
5. HistoryService now honors VT_HISTORY_DB_PATH, set in
deployment/compose_apps.yaml to live inside the existing vt_tool_cache
volume - history no longer gets wiped on every container recreate.
6. Documented MISPURL/MISPKEY for the Docker deployment (deployment/.env.example,
deployment/README.md) and the History/MISP-push flow (vt-tool-ui/README.md).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
.env.example ships VT_CACHE_TTL_HOURS= (present, empty), so
os.getenv's two-arg default never kicks in and float('') raised,
crashing cache setup for every entrypoint (CLI, API, worker).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ValueReader stored args.values in __init__ but read_values() never
read it - positional CLI values (e.g. `vt_tools.py example.com`)
were silently dropped, analyzing nothing.
Fixing that exposed a second, pre-existing bug: when stdin is open
but not a tty and yields zero lines (any non-interactive/automated
run with no terminal attached - exactly -n mode's use case),
read_from_stdin() returns a bare {} with no keys at all.
_combine_and_clean_values only iterated the first dict's keys, so
it silently dropped everything from the second dict and later
raised KeyError('domains') downstream. Now unions both dicts' keys.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A full-codebase audit surfaced several real, previously-latent bugs. All fixed with regression tests: - Env vars present-but-empty (`KEY=` in .env) silently bypassed os.getenv's two-arg default, same class of bug already hit once today (VT_CACHE_TTL_HOURS). Added a shared get_env() helper (app/DataHandler/utils.py) and applied it everywhere this pattern existed: REDIS_URL (crashed API/worker startup), VT_HISTORY_DB_PATH (silently opened a temp DB that vanished on restart), VTSSLVERIFY (silently disabled TLS verification), CORS_ALLOWED_ORIGINS (degraded to an empty allow-list instead of the wildcard default). - Template mode (-tf) called exit() on every use, raising SystemExit uncaught by `except Exception` - now degrades gracefully instead of hard-crashing the process. - The hourly-quota guard in analyze_value_type never decremented its counter, so a large batch could blow through real VT quota instead of stopping mid-run as the printed warning promised. - VirusTotalService distinguished "not found" from other API failures by substring-matching the exception message - now checks vt.APIError's structured .code field, so a transient error that happens to mention that substring can't get miscategorized and cached as a permanent false negative. - The MISP push endpoint's history bookkeeping ran outside the try/except wrapping the actual push, so a failure there (e.g. a sqlite lock) could return a raw 500 even after MISP already accepted the objects, risking a client retry that re-pushes them. - MispService.create_object() swallowed per-item errors with only a server-side log line; the push endpoint now collects and returns skip_reasons so a systematic misconfiguration doesn't look like an opaque "N skipped". - get_remaining_quota() had no error handling around JSON parsing, unlike the request-exception path right above it - a malformed 200 response used to crash the whole run instead of degrading gracefully. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
useJobPolling's shouldKeepPolling only looked at the last
successful fetch's data, ignoring the query's error state - a
permanently-failing /jobs/{id} got polled every 2s forever with no
cap or backoff. Now stops once the query settles into an error
state (with a bounded retry count), and shouldKeepPolling is
exported for direct unit testing.
Also updates MispPushResult's TS type for the backend's new
skip_reasons field (see the backend audit-fix commit) across the
API client, hooks, and e2e mocks.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
init_client() returned False on failure (bad/empty API key, or any
other vt.Client() construction error), but both callers (the CLI's
Initializator and the API worker's job task) used the return value
as if it were a real client - .get_object()/.close() on a bool
produced a confusing "'bool' object has no attribute 'close'"
instead of the actual cause.
Root cause of the reported crash: the web UI submitted an analysis
with no VirusTotal API key configured (Settings page never set),
so vt.Client("") raised ValueError, which used to surface as that
AttributeError. Now raises VirusTotalAPIError with the real reason,
which the worker job/API's /jobs/{id} endpoint already surfaces as
a clear per-row error message.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The "no API key" banner was advisory only - a user could still click Analyze with no key configured, silently submitting an empty key and hitting a confusing backend crash instead of ever being told why (see the VirusTotalClient fix in 223e4ce). IocReviewTable's Analyze button now takes a disableSubmit prop, wired to !hasApiKey in AnalyzePage. The banner is also bumped from a warning to an error severity now that it reflects an actual block, not just a suggestion. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI's fresh pip install resolves the latest fastapi/starlette (starlette 1.6.0), whose TestClient requires httpx (or httpx2, tried first) - neither was declared anywhere, so tests/test_api.py failed to import in CI with RuntimeError. Passed locally only because a dev venv happened to already have httpx installed from something else, masking the gap. Verified in a genuinely fresh venv. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
vt-tool-ui/, a React SPA over vt_tool's existing FastAPI/arq backend, plus a batch of backend robustness fixes found while building and load-testing it.Frontend (
vt-tool-ui/)get_misp_event/submit_misp_objects, credentials stay server-side, never touch the browser).deployment/'s Compose stack alongsidevt-tool-api/vt-tool-worker/redis.Backend
CORSMiddlewareadded to the API (the only backend change v1 required).HistoryService(independent SQLite store) andPOST/GET /analyses,GET /analyses/{id},POST /analyses/{id}/misp-pushendpoints.VirusTotalClient.init_client()now raises a clear error instead of returning a bool sentinel that got misused as a real client downstream (was producing a confusing'bool' object has no attribute 'close'crash on a bad/empty API key).ValueReadernow actually reads CLI positional values (previously silently dropped) and no longer loses everything when stdin is open-but-empty.get_env()helper fixes several env vars that were present-but-blank silently bypassing their defaults (VT_CACHE_TTL_HOURS,REDIS_URL,VT_HISTORY_DB_PATH,VTSSLVERIFY,CORS_ALLOWED_ORIGINS) - one previously disabled TLS certificate verification silently.-tf) no longer hard-crashes the process via an uncaughtexit().git logon this branch has the full history, each commit is self-contained with its own rationale).Test plan
ruff check .clean,python -m unittest discover -s tests -t .- 245/245 passingpnpm test- 81/81 passing,pnpm run buildclean,pnpm run lintclean🤖 Generated with Claude Code