Skip to content

feat: five-bucket app improvement pass (correctness, ATT&CK, HTTP analysis, API, CI) - #14

Closed
ninedter wants to merge 35 commits into
mainfrom
claude/app-improvement-review-e2fb60
Closed

feat: five-bucket app improvement pass (correctness, ATT&CK, HTTP analysis, API, CI)#14
ninedter wants to merge 35 commits into
mainfrom
claude/app-improvement-review-e2fb60

Conversation

@ninedter

Copy link
Copy Markdown
Owner

Summary

Implements all five improvement buckets from the 2026-07-13 app-wide review, executed subagent-driven (an implementer + an independent reviewer per task, with fix loops). 34 commits; full test suite green (1196 passed, 0 failed, 22 skipped), coverage 60.7% ≥ 58% gate, ruff clean.

Bucket 1 — Correctness batch

  • Zeek in-memory tables were hardcoded-truncated to 2,000 rows with no signal → now a ZEEK_TABLE_MAX_ROWS (50k) cap that emits WARNING_ZEEK_TRUNCATED.
  • IOC-feed STIX mislabeled every hash as SHA-256 (and dropped JA3, mislabeled IPv6) → routed through a shared, length-aware ioc_to_stix_pattern; fixed a third stale copy in ioc_export.py too.
  • cases.db now sets WAL + busy_timeout (every other SQLite store already did) — prevents "database is locked" under concurrent worker/API/UI writes.
  • The PDF report's attack-timeline chart was dead (session key never written) → persisted at pipeline completion, with a stale-leak guard on re-run/restore.
  • OSINT enrichment leaked internal domains (*.internal, *.local, *.in-addr.arpa, IP-shaped) to VirusTotal/OTX → is_enrichable_domain filter.

Bucket 2 — MITRE ATT&CK wired end-to-end

The ATTACKMapper engine was fully built but never instantiated. Now: built in the runner, populates PipelineResult.mitre_techniques + a persisted attack_mapping, rendered in the Dashboard, added as a PDF section, and the IOC feed's mitre_techniques is derived from a new analysis_techniques table (was always []). Deduped a contradictory inline JA3 attribution against the authoritative ja3.py. (Wiring it live also surfaced and fixed a real _check_tls crash on the actual analyze_certificates output shape.)

Bucket 3 — HTTP analysis stage + HTTPS-beacon tuning

  • New analyze_http stage (consumes the already-collected but previously-unused http.log): suspicious user-agents, cleartext credentials, suspicious URIs; wired into the runner, correlation (new Tier-2 signals), UI, and persistence. Password values are never emitted.
  • The blanket ×0.15 port-443 beacon penalty (which buried real HTTPS C2) is now conditional — softened only for very regular, high-confidence, small-payload flows (so bulk CDN keep-alives stay suppressed), and decoupled from the ATT&CK C2 threshold so a benign periodic flow can't auto-escalate to a high-severity false positive.

Bucket 4 — API additions (under /api/v1)

Case CRUD (list / PATCH / notes), an exact-match single-IOC /iocs/lookup, a CEF/syslog IOC feed (/iocs.cef), and an optional SSRF-guarded completion webhook on submit (blocks private/loopback/link-local/multicast, refuses redirects, HMAC-signable, never flips job state on failure). Also hardened CEF output against CRLF log-injection. Docs updated in docs/API.md.

Bucket 5 — CI hardening + first-run experience

Coverage floor (--cov-fail-under=58), pip-audit job + Dependabot, a Docker-build-and-test CI job (runs the suite with Zeek, closing that CI gap), a committed synthetic demo pcap (DNS/HTTP/beacon) + a "Load demo capture" button, SECURITY.md + CONTRIBUTING.md, and a refreshed roadmap. Also de-flaked 6 pre-existing environment-fragile tests (stale hardcoded date + timezone regex) so the suite is green on host and CI.

Testing

  • PYTHONPATH=. pytest tests/ --cov=app --cov-fail-under=58 → 1196 passed, 0 failed, 22 skipped, coverage 60.7%.
  • ruff format --check . and ruff check . clean.
  • Every task was independently reviewed; a final multi-agent whole-branch review found no must-fix blockers, and its four cross-task findings (batch under-reporting, timeline-restore regression, a beacon→ATT&CK false-positive, host:port signal drop) were fixed.

Deliberately deferred (documented, non-blocking)

  • Webhook DNS-rebinding TOCTOU (mitigated today by the full-scope-key requirement; a resolve-and-pin adapter is future hardening).
  • Future detection ideas listed in the refreshed docs/FEATURE-ROADMAP.md (lateral-movement/SMB, JA4/JARM, IPv6 OSINT enrichment, MISP/Sigma export, multi-user/RBAC + audit trail, retention policy).

🤖 Generated with Claude Code

ninedter added 30 commits July 13, 2026 19:58
cases.db is the hottest SQLite store in the app (pool workers, API
thread, and Streamlit UI all write concurrently) yet was the only
store without journal/timeout pragmas, unlike key_repository.py,
osint_cache.py, and rdns_cache.py. Sets WAL once in _init_schema and
busy_timeout=30000 per-connection in _get_conn, matching the existing
convention. foreign_keys remains OFF -- delete_case/clear_all rely on
explicit manual cascade.
IOCExporter._ioc_to_stix_pattern was a third, stale copy of STIX-pattern
logic that dropped SHA-512 hashes, mislabeled IPv6 as ipv4-addr, and
always returned None for JA3 IOCs. Delegate to the shared
app.utils.stix_export.ioc_to_stix_pattern helper instead.
…_ja3

_check_ja3_from_features carried its own inline known_malware_ja3 dict
that contradicted app.pipeline.ja3's KNOWN_JA3_FINGERPRINTS -- the same
hash was attributed to two different malware families (e.g. TrickBot
here vs. Cobalt Strike there), eroding analyst trust in attribution.
Route through lookup_ja3() instead, deriving confidence from its
authoritative severity field rather than a flat constant.
Instantiate ATTACKMapper post-fan-out in run_pipeline() and populate
PipelineResult.mitre_techniques/attack_mapping; a mapper failure is caught
and logged so it never breaks the pipeline. Persist attack_mapping to a new
analyses.attack_json column (idempotent ALTER, same compress/decompress
pattern as dns_json/tls_json).

Fixes a latent _check_tls bug found while wiring this up for the first
time: analyze_certificates() reports per-cert is_self_signed/is_expired
flags in a certificates list plus aggregate counts in alerts, not a list
of {type, cert} alert objects — the mapper was dead code until now so this
never fired in production. Updated the test in test_phase4.py that had
encoded the fictitious shape as expected behavior, and added
tests/test_attack_mapping.py with production-shape regression coverage.
Populate PCAPResult.attack_mapping from the runner result and store it in
session state for single-file, batch, and case-restore/save UI flows; set
Analysis.attack_mapping in the API worker's persist step so the queue path
saves it too (DB column/round-trip already added in task 2.2).
session_state["attack_mapping"] holds the dict form (AttackMapping.to_dict())
but render_attack_mapping expects the object form. Add from_dict classmethods
on AttackMapping/TechniqueMatch (empty/partial dict safe) to reconstruct
before rendering, wire the call into the Dashboard tab between the
correlation and hunting-checklist sections, and seed attack_mapping in the
boot-time session-state defaults for symmetry with the click-reset block.
New analysis_techniques table stores per-analysis MITRE ATT&CK technique
IDs (full-replace on re-save, like attack_json). The IOC feed's query_iocs
now LEFT JOINs it and aggregates via GROUP_CONCAT(DISTINCT ...), replacing
the always-empty mitre_techniques field. delete_case and clear_all cascade
the new table; queue.py's _persist_analysis sets Analysis.mitre_techniques
from the pipeline result before saving.
Both manual-save paths in cases_tab.py (_quick_save_analysis and
_add_current_analysis_to_case) built Analysis objects without
mitre_techniques, so cases saved via the UI shipped a permanently-empty
IOC feed mitre_techniques field even though the API/queue path already
derives it from result.mitre_techniques. Add a shared
_session_mitre_techniques() helper that derives technique IDs from the
session's attack_mapping dict and wire it into both save paths.
…lows

The blanket 0.15 multiplier on port 443 (BENIGN_SERVICE_PORTS) crushed a
genuinely periodic HTTPS beacon scoring 0.9-1.0 raw down to ~0.135-0.15,
below both BEACON_SCORE_THRESHOLD (0.6) and the correlation ingest gate
(0.5), making real HTTPS C2 -- the dominant C2 transport -- invisible.

Make the penalty conditional: only when a 443 flow is both very
high-confidence (raw score >= 0.85) and essentially jitter-free
(jitter_pct <= 15) does it get a softened 0.7 multiplier instead of 0.15,
tuned so a perfectly periodic flow (raw ~1.0) clears threshold with
margin (1.0 * 0.7 = 0.7 > 0.6). A naive 4x multiplier (0.15 -> 0.6) was
considered and rejected -- it still leaves a 0.9-raw flow at 0.54, under
threshold. Ordinary jittery HTTPS keep-alives fail the >=0.85 gate and
keep the full penalty, so they stay suppressed.

Restricted to port 443 only (SOFTENABLE_BENIGN_PORTS) -- DNS/NTP/IMAPS/etc.
in BENIGN_SERVICE_PORTS are genuinely periodic infrastructure by design,
not a C2 cover transport, so they keep the full penalty regardless of
regularity.

Considered relaxing the correlation.py:129 beacon_lookup > 0.5 gate as an
alternative/additional fix, but left it untouched: a softened-but-real 443
beacon now scores above 0.5 on its own and passes that gate without
further changes.
Adds the HTTP analysis stage (app/pipeline/http_analysis.py, from a prior
commit) to the concurrent post-parse fan-out in app/pipeline/runner.py,
gated on http.log being present in zeek_tables. Registers the "HTTP
Analysis" phase in main.py's two phase lists and session-state
defaults/reset blocks for consistency with the other zeek-log stages.
Adds GET /api/v1/iocs/lookup for the #1 SOAR enrichment pattern (is this
IP/domain/hash known, and its score) so clients no longer need to pull
the whole feed and filter locally.

- IOCFilter gains an optional value field; query_iocs adds an exact
  'AND i.value = ?' predicate (uses idx_iocs_type_value), composing with
  the existing type/tag/case_id filters and mitre_techniques join.
- New route reuses IOCEntry/IOCFeedResponse as response_model (previously
  declared but unused); cursor pagination is skipped since a single value
  aggregates to at most one row.
Add GET /api/v1/iocs.cef for ArcSight/QRadar/Sentinel-style SIEM ingestion,
backed by a new feed_rows_to_cef() adapter in cef_export.py. The existing
_events_from_iocs() expects ScoredIOC-shaped priority_score (0.0-1.0) and
drops anything below 0.4; feed rows carry an integer score (25/50/75/100)
instead, so a naive score/100 mapping would silently drop LOW-severity IOCs
from a feed pull. feed_rows_to_cef emits one CEF event per row with no
gating, reusing the existing _SEVERITY_MAP and CEFEvent/format_syslog
escaping logic. The route mirrors iocs.csv: same _build_filter query params,
same _conditional_response ETag/Last-Modified handling.
SOAR clients can now pass webhook_url on POST /api/v1/pcaps instead of
busy-polling GET /jobs/{id}. The worker subprocess POSTs a small envelope
(job_id, case_id, status, analysis_id) to the URL on both terminal states
(done/failed), with retry and optional HMAC signing.

is_safe_webhook_url (app/utils/network_utils.py) is the only SSRF guard,
since hardened_session does not itself block private IPs: it requires
http/https, resolves the host, and rejects if any resolved address is
not public (loopback/private/link-local/reserved/multicast/unspecified),
closing a DNS-rebinding-style bypass. The router validates at submit time
(422 on an unsafe URL); the worker's _dispatch_webhook re-validates
immediately before every POST as defense in depth, and never raises so a
webhook failure can't affect the job's own terminal status.
Measured total coverage is 61% (12239 statements, 1189 passed / 22
skipped, 0 failed). Set --cov-fail-under=58 (measured minus 3 points)
on the CI test step and the Makefile test target so CI enforces a
floor without flaking on minor Linux/branch-coverage differences vs
this host run. Also add [tool.pytest.ini_options] (permissive
filterwarnings) and [tool.coverage.report] (exclude_lines for
pragma/TYPE_CHECKING/NotImplementedError) to pyproject.toml.
ninedter added 5 commits July 14, 2026 04:14
…t excludes

Bare '*.pcap' currently only matches root-level files in the installed
BuildKit version (verified empirically), so pcaps/demo.pcap already
survives the runtime image build context. Add the negation anyway,
mirroring the .gitignore fix, so a future tightening to a recursive
pattern (e.g. '**/*.pcap') can't silently break the demo capture COPY.
Lets a first-run user try the pipeline immediately via a one-click
"Load demo capture" button on the Upload tab, wired to the committed
pcaps/demo.pcap. Also repoints scripts/capture_screenshots.py's default
sample pcap from the never-committed data/sample.pcap to pcaps/demo.pcap
so the screenshot flow works out of the box.
@ninedter ninedter closed this Jul 14, 2026
@ninedter
ninedter deleted the claude/app-improvement-review-e2fb60 branch July 14, 2026 10:56
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