Model updates for bank fraud reporting and bank identifier migrations (SYN-10665) - #5008
Merged
Conversation
…:bic (SYN-10665) Model updates needed by the synapse-cybera power-up backport from 3.x. Added ``risk:threat:ext:ids`` as an alts array for ``:ext:id``, mirroring ``meta:cluster:id``/``:ids``, so a threat cluster resolves from any of its member identifiers. Added ``risk:attack:actor`` typed ``risk:threat``. In 3.x this property comes from the ``entity:action`` interface and is typed ``entity:actor``; 2.x has no such interface, so it is deliberately narrower here. Updated the ``econ:bank:swift:bic`` regex to the 3.x pattern, which accepts the 8 character ISO 9362 head-office form. The 8 character form was previously rejected outright. The 3.x pattern also anchors the end. Since 2.x ``Str.norm`` uses ``regex.match()``, the previous pattern accepted trailing characters after a valid BIC, so this narrows the type and requires a migration for data at rest. That migration is not in this commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5008 +/- ##
==========================================
- Coverage 97.84% 97.84% -0.01%
==========================================
Files 308 308
Lines 65593 65696 +103
==========================================
+ Hits 64180 64280 +100
- Misses 1413 1416 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…-10665) Anchoring the end of the ``econ:bank:swift:bic`` regex narrows a released type, so values normed under the previous pattern may no longer re-norm. The previous pattern was anchored only at the start, because ``Str.norm`` calls ``self.regex.match()``, so it accepted trailing characters after a valid BIC. A trailing character BIC has no derivable correct value, so the migration removes the node rather than moving it. Each removed node is recorded in the ``model_0_2_37:nodes`` queue with its properties, tags, nodedata and edges, so an operator can rebuild it with the correct value. Unlike the 0.2.31 and 0.2.35 migrations, no node's primary value changes here: the new pattern accepts every valid BIC the old one did. No property in the model is typed ``econ:bank:swift:bic`` either, so there is no re-keying and no inbound reference to rewrite. No operator stormlib helper, following 0.2.35 rather than 0.2.31. Repairing a BIC requires a human supplying the correct code, and the queue is reachable through ``$lib.queue.get()``. Requires the ``model-0.2.37`` cortex from vertexproject/synapse-regression#33. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
Extends the 0.2.37 model revision to the other two bank identifier types,
so all three match Synapse 3.x:
econ:bank:aba:rtn [0-9]{9} -> ^[0-9]{9}$
econ:bank:iban [A-Z]{2}[0-9]{2}[a-zA-Z0-9]{1,30} -> ^[A-Z]{2}[0-9]{2}[a-zA-Z0-9]{1,30}$
Unlike the ``econ:bank:swift:bic`` change these are pure narrowings, with
no widening half. ``Str.norm`` uses ``regex.match()``, so the previous
patterns accepted trailing characters after an otherwise valid value.
Unlike the BIC, ``econ:bank:account`` has properties typed for both of
these, so removing a node now clears inbound references. Neither property
is read-only, so ``removeNode()`` deletes the property and the account
survives rather than being removed with the identifier. The queued record
retains the referring node and the property which pointed at it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
The 0.2.37 migration removes nodes as its only outcome, so its queue is expected to hold entries. That matches the 0.2.31 migration, which has a repair library, and not 0.2.35, whose ``removeNode()`` calls are both defensive branches marked ``# pragma: no cover``. Repair matters more here than the node contents alone suggest. The ``econ:bank:account`` properties which pointed at a removed identifier are cleared by the migration, and ``repairNode()`` re-points them at the corrected value. Rebuilding the node by hand would restore the node but silently leave the account without its identifier. Extracted ``LibModelMigrationsQueue`` so both libraries share the implementation and differ only by ``queuename``. The three ``_meth`` entry points are declared on each subclass rather than inherited because ``registry.registerLib()`` stamps ``_storm_funcpath`` onto the function object: with inherited methods both libraries resolve to the same object, and the second registration relabels the first. That would have renamed the released 0.2.31 library in its own error messages. Also covers the read-only referrer cascade added to the regression cortex in vertexproject/synapse-regression#33, and switches the Storm boolean literals in this file from ``$lib.true`` to ``(true)``. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
Repairing a node which had a read-only referrer takes two ``repairNode()`` calls: ``_repairNode()`` cannot re-point a read-only property, so the referring node was removed and queued under its own entry. Nothing told the operator the second call existed, and the referring node's queued ``formvalu`` embeds the old broken value, so it cannot simply be replayed. ``printNode()`` already prints the refs list and had ``isro`` available in ``refinfo`` but discarded it. Marking the read-only entries surfaces the follow-up where someone reading a record before repairing it is already looking. This applies to the 0.2.31 library as well, which has the same two step repair. Also covers the two layer node added to the regression cortex in vertexproject/synapse-regression#33, and reverts the ``$lib.true`` changes on the two 0.2.31 test lines this PR does not otherwise touch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
The ``econ:bank:swift:bic`` form exists but has nowhere to hang off an account, while ``:aba:rtn`` does. In a captured CYBERA feed sample the BIC is the more common of the two: 670 of 1216 records carry one against 384 with an ABA routing number. Placed beside ``:aba:rtn`` rather than at the end of the identifier properties. Both identify the bank which issued the account, where ``:number`` and ``:iban`` identify the account itself. This is not a property to property copy for a 3.x migration, because 3.x has no ``:swift:bic`` anywhere -- routing lives in the primary value of the ``econ:bank:account`` comp, and ``econ:bank:swift:bic`` is one of the three forms implementing ``econ:bank:routing:code``. The gain is that the account to BIC pairing becomes machine readable rather than only inferable through ``:issuer`` and the bank's ``:business`` properties, which is ambiguous once a bank has more than one BIC. That makes the BIC exactly as portable as ``:aba:rtn`` already is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
The test values named the third party feed the modeling work came from. Nothing depended on the string, and ``REPORT-`` describes what they stand for more directly: several report identifiers resolving to one threat cluster, which is what the alts array exists for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
mikemoritz
marked this pull request as ready for review
September 1, 2026 19:40
OCBender
reviewed
Sep 1, 2026
OCBender
previously approved these changes
Sep 1, 2026
invisig0th
reviewed
Sep 1, 2026
``getNode()`` only ran its ``setdefault()`` calls when the stored node was empty, which assumed every writer stores a fully populated dict. ``editNodeAdd()`` does not: it stores six of the keys, omitting ``refs``, ``layers`` and ``verdict``. A ``getNode()`` after that would skip the defaults and return a dict which raises ``KeyError`` on ``node['layers']``. The case is currently unreachable, because ``editNodeAdd()``'s only caller is ``moveNode()``, which populates the destination through ``getNode()`` and ``nodes.set()`` before calling it. That is a non-local invariant holding up a local guard, and ``setdefault()`` is idempotent, so dropping the guard costs nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
``entity:actor`` already exists in 2.x as an ndef over ``ou:org``, ``ps:person``, ``ps:contact`` and ``risk:threat``, and is already used by ``entity:relationship:source`` and ``:target``. Typing ``:actor`` as ``risk:threat`` was needlessly narrow and would have required a migration to widen after release. This also matches 3.x, where ``risk:attack`` picks up ``:actor`` from the ``entity:action`` interface typed ``entity:actor``, so an attack attributed to an organization or a contact migrates as cleanly as one attributed to a threat cluster. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
invisig0th
approved these changes
Sep 2, 2026
OCBender
approved these changes
Sep 2, 2026
vEpiphyte
approved these changes
Sep 2, 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
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
Model updates to support ingesting bank fraud and scam reporting from a third party feed, plus the model revision the type changes require. These bring the 2.x model closer to 3.x for threat clustering and bank identifiers.
The
model-0.2.37regression cortex thattest_modelrev_0_2_37loads landed in vertexproject/synapse-regression#33.risk:threat:ext:ids:ext:idgains'alts': ('ext:ids',)plus the array property, mirroringmeta:cluster:id/:idsverbatim. The vendor mints one report id per report and a scam operator spans several, so the cluster collapses onto onerisk:threatand must resolve from any member id.risk:attack:actorTyped
entity:actor, the existing 2.x ndef overou:org,ps:person,ps:contactandrisk:threatwhichentity:relationship:sourceand:targetalready use. This matches 3.x, whererisk:attackpicks up:actorfrom theentity:actioninterface typedentity:actor. 2.xrisk:attackpreviously had no route torisk:threatat all --:attackeris aps:contact.Note it sits next to the deprecated
:actor:organd:actor:person. Those are unrelated and stay deprecated in favour of:attacker.econ:bank:account:swift:bicThe
econ:bank:swift:bicform existed but had nowhere to hang off an account, while:aba:rtndid. That asymmetry does not reflect real reporting, where the BIC is at least as common an account identifier as an ABA routing number.Placed beside
:aba:rtn, since both identify the bank which issued the account, where:numberand:ibanidentify the account itself.This is not a property-to-property copy for a future 3.x migration -- 3.x has no
:swift:bicanywhere, because routing lives in the primary value of theecon:bank:accountcomp andecon:bank:swift:bicis one of three forms implementingecon:bank:routing:code. The gain is that the account-to-BIC pairing becomes machine-readable rather than only inferable through:issuerand the bank's:businessproperty, which is ambiguous once a bank has more than one BIC. It makes the BIC exactly as portable as:aba:rtnalready is.Bank identifier types
All three now match 3.x exactly:
econ:bank:aba:rtn[0-9]{9}^[0-9]{9}$econ:bank:iban[A-Z]{2}[0-9]{2}[a-zA-Z0-9]{1,30}^[A-Z]{2}[0-9]{2}[a-zA-Z0-9]{1,30}$econ:bank:swift:bic[A-Z]{6}[A-Z0-9]{5}^[A-Z]{6}[A-Z0-9]{2}([A-Z0-9]{3})?$The BIC change is the one motivating this work: the 8 character ISO 9362 head-office form is entirely normal and was rejected outright, so values such as
BOPIPHMMandTRWIBEB1raisedBadTypeValu. The other two are pure narrowings, aligned so the two model versions agree on what these identifiers are.Model revision 0.2.37
Str.normcallsself.regex.match()(synapse/lib/types.py:1862), which anchors only at the start. All three previous patterns therefore accepted trailing characters after an otherwise valid value --DEUTDEFFXXXXX,1234567890andGB29NWBK60161331926819!!all normalized. Anchoring the end narrows released types, so data at rest may no longer re-norm.None of these values has a derivable correct replacement, so
ModelMigration_0_2_37removes the node rather than moving it. Each removal is logged at WARNING with the node's iden, and the full record -- properties, tags, nodedata, both edge directions, and the referring nodes -- goes into themodel_0_2_37:nodesqueue so an operator can rebuild it. The queue is created lazily on first write, so a Cortex with nothing to migrate never gets one.It is smaller than the two existing primary-value migrations because no node's primary value changes: every valid value the old patterns accepted is still accepted.
moveNodeis never called, which also leaves the base class's recursive comp-form re-key path dormant.econ:bank:accountdoes have properties typedecon:bank:aba:rtnandecon:bank:iban, so inbound references are cleared. Neither property is read-only, soremoveNode()deletes the property and the account survives rather than being removed with the identifier. A read-only referrer is removed and queued under its own record instead; the regression cortex covers that arm with ameta:seen.Repair library
$lib.model.migration.s.model_0_2_37provideslistNodes(),printNode()andrepairNode(), matching the existingmodel_0_2_31library.This follows
_0_2_31rather than_0_2_35. The distinction is not whether a migration writes a queue but whether the queue is ever expected to be non-empty: both of_0_2_35'sremoveNode()calls sit insideexcept Exception: # pragma: no coverand its normal path ismoveNode(), so its queue is expected to stay empty and no test asserts it. Removal is this migration's only outcome.Repair matters more here than the node contents suggest. The
econ:bank:accountproperties which pointed at a removed identifier are cleared by the migration, andrepairNode()re-points them at the corrected value -- rebuilding the node by hand would restore the node but silently leave the account without its identifier.listNodes()'sformfilter, inert on_0_2_31's single-form queue, is useful on a three-form one.printNode()marks read-only references, becauserepairNode()cannot re-point a read-only property -- the referring node was removed and queued under its own entry, so restoring it is a second call that was previously undiscoverable. This improves the_0_2_31library too, which has the same two-step repair.LibModelMigrationsQueuewas extracted so both libraries share the implementation and differ only byqueuename. The three_methentry points are declared on each subclass rather than inherited:registry.registerLib()stamps_storm_funcpathonto the function object, so with inherited methods both libraries resolve to the same object and the second registration relabels the first -- which would have renamed the released_0_2_31library in its own error messages.Tests
test_model_risk.py--:ext:id/:ext:idswith strip applied, the alts resolverisk:threat=({"ext:id": <peer id>}), and:actorset to both arisk:threatand anou:orgwith pivots to eachtest_model_economic.py-- BIC 8 character accepted, 7/9/10/13 rejected; RTN and IBAN trailing-character cases rejected;:swift:bicset on an account with a value assertion and a-> econ:bank:swift:bicpivottest_lib_modelrev.py-- the repair library (form-filtered listing,printNodeshowing the referring property and flagging read-only refs,repairNoderestoring both the node and the account property, and the queue-absent messages), plustest_modelrev_0_2_37in the usual two-block form. The quarantined BIC spans two layers, so the record carries a sode and nodedata for each and the migration's missing-valuguard is exercised:maxvers=(0, 2, 36)asserting the pre-migration state, then a full boot asserting the survivors keep their properties, all six invalid values are gone (one from a forked view), the referencing account survives with:numberintact and both identifier properties cleared, and each queued record carries the referring node plus the property which pointed at ittest_lib_modelrev.pyis 36 passed / 0 skipped withSYN_REGRESSION_REPOset, and every other regression cortex migrates cleanly through the new step. The no-invalid-nodes path -- what almost every real Cortex will take -- is covered by the existingmodel-0.2.36fixture.Changelog
A
migrationfragment covering the removals, the cleared account properties and the read-only cascade, plus afeatfragment for the new Storm library. No fragments for the model changes themselves --ModelDifferpicks up all of them against the v2.250.0 model ref (both new properties, theext:idaltsaddkey, and all three types with old and new regexes), and they are covered by the generated model update doc.🤖 Generated with Claude Code
https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs