RFC-019: Threat Analyst Agent Engine Primitives (ZF scope of Spec 011) - #193
Conversation
Extracts the ZF-scoped requirements from the Spec 011 Threat Analyst Agent expansion package (2026-07-07, rev 1.2) into a ZettelForge RFC: IoC normalization/refanging, provenance-complete enrichment with an explicit-absence contract, bounded deterministic subgraph payloads, closed-form indicator scoring with per-type decay and sightings, and staged/reversible ingestion primitives (span-anchored free-text import, dedup, warning lists, correlation candidates). TR-tagged concerns (tenant policy, feeds, SIRP adapters, UI) are explicitly out of scope. E-FR IDs preserved for traceability to the source package. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqYpmV8TABMzq27HBTLtNP
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f22e13b23b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| existing pattern of one `MemoryManager`/store per tenant (as `web/auth.py`'s | ||
| `get_mm_for_request` already does); no tenant logic enters engine primitives. |
There was a problem hiding this comment.
Stop treating the auth stub as tenant-isolated
I checked the current web/auth.py:get_mm_for_request implementation: it is explicitly a single-tenant stub and returns the module-global _default_mm for every request, not one MemoryManager/store per tenant. If this RFC is used as implementation guidance, treating E-FR-026 as already satisfied here can make multi-tenant callers share the same committed corpus and leak enrichment/graph results across tenants; the RFC should require an actual tenant pre-filter/callback or avoid citing the stub as satisfying isolation.
Useful? React with 👍 / 👎.
| Validator rules: the document slice `[char_start, char_end)` must equal | ||
| `verbatim_span` byte-for-byte, and `canonical` must equal |
There was a problem hiding this comment.
Use one unit for span offsets and validation
E-FR-035 defines char_start/char_end as character offsets, but this validator then requires byte-for-byte equality. For source reports containing UTF-8 characters before or inside the span, byte offsets and character slices diverge, so a literal implementation either rejects valid regex/LLM spans or stores offsets that UI callers cannot round-trip. Please specify exact character-slice equality, or rename these to byte offsets and define the encoding.
Useful? React with 👍 / 👎.
| Deterministic ordering rule (normative): nodes sorted by | ||
| `(stix_type, id)`; edges by `(relationship, src, dst)`. Truncation removes | ||
| highest-index entries under that ordering, never a random subset. |
There was a problem hiding this comment.
Add a stable tie-breaker for graph edges
I checked the current SQLiteBackend.add_kg_edge: it dedupes on (from_node_id, to_node_id, relationship, note_id), so two notes can create parallel edges with the same relationship/src/dst but different provenance. In that multi-source case this normative sort key ties, leaving the output order dependent on backend iteration and breaking E-FR-013's byte-identical payload guarantee; include a stable tiebreaker such as edge_id or provenance ref.
Useful? React with 👍 / 👎.
| | E-FR-032 | **Staging and batch model**: all external input lands in staging keyed by an import batch ID. Commit is an explicit, audited transition after normalization, dedup, and quality gates. Every committed object carries batch provenance; a whole batch is revertible — revert removes or tombstones exactly that batch's objects and derived correlation candidates. Committed objects are immutable; corrections flow through retraction or supersession, never in-place edits. | ZF storage | | ||
| | E-FR-034 | **Normalization unification**: the import path calls the same §4 implementation as the query path. STIX 2.1 field mapping, UTC timestamps, confidence mapped to one scale, TLP 2.0 markings preserved. | ZF | | ||
| | E-FR-035 | **Free-text import**: extract candidate observables/entities from unstructured text. Every proposal is span-anchored (source doc ref, character offsets, verbatim span). A validator rejects any proposal whose span does not exact-match the document slice or whose canonical form does not derive from the span via §4. LLM-assisted extraction is permitted **only behind this validator**: the extractor proposes, the validator disposes. | ZF | | ||
| | E-FR-036 | Staged proposals require approval before commit (approval *policy* is the caller's; the staging/approval state machine is the engine's). Committed objects carry document-plus-span provenance. | SHARED | | ||
| | E-FR-037 | **Dedup at commit**: exact canonical-form and STIX ID matches merge automatically with a merge audit entry preserving both provenance chains. Similarity-based near-duplicates are flagged as merge candidates and never auto-merged. | ZF | |
There was a problem hiding this comment.
Preserve pre-existing objects when reverting merged batches
When a batch contains an indicator already present in the corpus, E-FR-037 says the exact duplicate is auto-merged, but E-FR-032 still says revert removes or tombstones exactly that batch's objects. A literal revert could tombstone the shared merged object and delete pre-existing intelligence from another batch; the RFC should specify rolling back only the batch's provenance/contribution while leaving the prior object intact.
Useful? React with 👍 / 👎.
| "attack_annotations": [ | ||
| { "technique_id": "T1071.001", "tactic": "command-and-control", | ||
| "attack_version": "<pinned>" } | ||
| ] |
There was a problem hiding this comment.
Include retrieval explanation in enrichment schema
E-FR-002 requires every enrichment response to include a retrieval-explanation block, but the only response schema sketch ends after attack_annotations with no such field. Implementers and tests generated from this sketch can omit a contract-required field while appearing schema-compliant; add the retrieval_explanation member here or mark it explicitly optional in E-FR-002.
Useful? React with 👍 / 👎.
| score(t) = clamp_0_100( base_confidence * decay(t - anchor, type) | ||
| + sum_i( w(disposition_i) * s_i * decay(t - t_i, type) ) ) | ||
| w(true_positive) > 0, w(false_positive) < 0, w(unknown) = 0 |
There was a problem hiding this comment.
Define the scoring variables and weights
The score is described as deterministic and CI-testable, but the formula uses s_i without defining it and only constrains the disposition weights by sign. For the same sightings, different implementations could treat s_i as count, source confidence, or a capped signal and choose different weight magnitudes, producing incompatible scores; make these explicit model parameters and include them in the explanation payload.
Useful? React with 👍 / 👎.
| |---|---|---| | ||
| | E-FR-032 | **Staging and batch model**: all external input lands in staging keyed by an import batch ID. Commit is an explicit, audited transition after normalization, dedup, and quality gates. Every committed object carries batch provenance; a whole batch is revertible — revert removes or tombstones exactly that batch's objects and derived correlation candidates. Committed objects are immutable; corrections flow through retraction or supersession, never in-place edits. | ZF storage | | ||
| | E-FR-034 | **Normalization unification**: the import path calls the same §4 implementation as the query path. STIX 2.1 field mapping, UTC timestamps, confidence mapped to one scale, TLP 2.0 markings preserved. | ZF | | ||
| | E-FR-035 | **Free-text import**: extract candidate observables/entities from unstructured text. Every proposal is span-anchored (source doc ref, character offsets, verbatim span). A validator rejects any proposal whose span does not exact-match the document slice or whose canonical form does not derive from the span via §4. LLM-assisted extraction is permitted **only behind this validator**: the extractor proposes, the validator disposes. | ZF | |
There was a problem hiding this comment.
Define validation for non-IoC free-text entities
This requirement covers both observables and entities, but the validator is tied to §4 canonicalization, which only type-infers IoCs (ipv4, domain, hashes, email, etc.). For free-text proposals like malware families, intrusion sets, tools, or victim identities, canonicalize(verbatim_span) has no defined output, so a literal implementation either rejects valid non-IoC entities or lets them bypass the grounding rule; add per-entity canonicalizers or scope this importer to IoCs only.
Useful? React with 👍 / 👎.
| Commit pipeline order per batch (normative): normalize (E-FR-034) → dedup | ||
| (E-FR-037) → warning-list and FP-list gates (E-FR-038) → marking propagation | ||
| (E-FR-047) → commit with batch provenance stamped on every object. |
There was a problem hiding this comment.
Gate warning lists before mutating dedup merges
This normative order runs dedup before warning-list and FP-list gates. For a staged indicator that exactly matches an existing corpus object and also hits a warning/FP list, E-FR-037's automatic merge can stamp batch provenance onto the committed object before the later gate excludes or flags it; make dedup non-mutating until gates pass, or move the gates before any merge write.
Useful? React with 👍 / 👎.
| | SC-3 | Determinism: ≥10 repeated identical graph queries against a fixed corpus snapshot produce byte-identical payloads in CI. | | ||
| | SC-4 | Decay correctness: computed scores match the closed form within 1e-6 across the type-parameter fixture matrix; expiry leaves the export evaluation within one refresh cycle. | | ||
| | SC-8 | Span grounding: 100% of committed free-text-derived objects carry spans that exact-match their source documents. Zero unanchored commits. | | ||
| | SC-9 | Item conservation: for the seeded import fixture, input count equals committed count plus audited exclusions and flags. Zero silent drops; every exclusion names its gate. | |
There was a problem hiding this comment.
Count warning-list flags separately
E-FR-038/039 allows warning-list hits to be flags rather than exclusions, so a flagged item may still be committed. This conservation formula adds committed count plus audited exclusions and flags, which double-counts flagged-but-accepted items and can make the seeded CI criterion impossible or encourage implementations to drop flagged data just to balance the equation; define mutually exclusive terminal buckets or track flags as attributes on committed/excluded items.
Useful? React with 👍 / 👎.
| |---|---| | ||
| | E-FR-025 | All new capabilities are exposed as MCP tools with published, versioned JSON schemas. **Any PR that adds, removes, or renames an MCP tool updates the tool documentation in the same PR** — the permanent control for tool-count drift. | | ||
| | E-FR-027 / E-FR-031 | No external network calls on any query-plane path. External fetch exists only on the ingestion plane, and even there the engine consumes bytes handed to it — actual feed pulling/scheduling is the caller's. | | ||
| | E-FR-028 | Every agent-facing response carries `schema_version` and the **corpus snapshot identifier**. New primitive: a monotonic write-generation counter maintained by the storage backend (bumped on every committed write/merge/retraction), sufficient for "unchanged corpus" determinism checks without content hashing. | |
There was a problem hiding this comment.
Pin reads to the reported corpus snapshot
A monotonic write-generation counter identifies writes, but it does not by itself make a multi-read enrichment or graph traversal come from one corpus snapshot. If a write/merge/retraction occurs while the operation is assembling nodes, edges, and provenance, the response can mix two generations while carrying only one corpus_snapshot, breaking the determinism and citation contract; require a read transaction or retry when the generation changes between start and finish.
Useful? React with 👍 / 👎.
Summary
New RFC extracting the ZF-scoped items from the Spec 011 Threat Analyst Agent expansion package (2026-07-07, rev 1.2) into a ZettelForge specification:
docs/rfcs/RFC-019-threat-analyst-engine-primitives.md. Docs-only — no implementation lands with this PR.Related issue
None — RFC submission for review/discussion. Depends on issue #47 (IPv6 extraction) as a phase-1 prerequisite once accepted.
Changes
docs/rfcs/RFC-019-threat-analyst-engine-primitives.md(new) — covers, with the package'sE-FRIDs preserved for traceability:ZF-tagged requirements plus the mechanism half ofSHAREDones are in scope; allTR-tagged concerns (tenant policy, feed registry/scheduling, SIRP adapters, UI render, RBAC) are enumerated as out of scope in §14.known: false) contract, conflict preservation, batch isolation, retracted-object handling.entity_indexer.py,knowledge_graph.py,sqlite_backend.pykg_nodes/kg_edges,mcp/server.py) and follows the RFC-016 storage precedent.Testing
pytest tests/ -v) — N/A, docs-only changeruff check src/zettelforge/) — N/A, no source changesmkdocs build— no new warnings (7 pre-existing, unchanged); RFC-019 intentionally not added to nav, matching RFC-007…018 conventionGenerated by Claude Code