feat(utils): ship the HL7v2 simulator as a workspace utility - #18
Open
ipasechnikov wants to merge 4 commits into
Open
feat(utils): ship the HL7v2 simulator as a workspace utility#18ipasechnikov wants to merge 4 commits into
ipasechnikov wants to merge 4 commits into
Conversation
Vendors HealthSamurai/hl7v2-simulator into utils/hl7v2-simulator so workspace users can generate synthetic HL7v2 traffic without an upstream system. Closes the "make it usable outside the team" half of interbox#93. It stays a self-contained package with its own package.json and lockfile rather than joining the root install: a workspace that never simulates traffic shouldn't carry faker and friends on every `bun install`. `bun run simulator` from the root installs it and opens the UI, already pointed at the MLLP port docker-compose publishes. Packaging changes on top of upstream: - Bind the UI to loopback by default (HOST to override). Bun.serve otherwise binds 0.0.0.0, and this server has no auth — /export writes and, with clean:true, deletes files at a path taken from the request body, and /probe opens TCP connections on request. Fine on a dev box, not something to hand to the network the moment we tell customers to run it. - Resolve default paths against the package root instead of the caller's cwd, so starting from the workspace root works and state doesn't scatter. - Make the hardcoded INTERBOX MSH-5/6 overridable via RECEIVING_APP / RECEIVING_FACILITY, for stands that route on the receiver fields. - Drop --hot from `bun run ui` (kept as ui:dev); the server holds a listener and per-source actors that a reload leaks. - Move typescript from peer to dev deps so the new CI job can typecheck. Root wiring: bunfig.toml scopes `bun test` to test/ (bare `bun test` would otherwise collect the simulator's suite, which can't resolve imports without that package's own install), and CI gets a separate job for it. Also corrects two stale README claims found while documenting: `bun run bundle` is not a script (the engine bundles at boot), and the simulator's `--load` flag has no handler.
The workspace ingests more than HL7v2 — the engine also has CSV and folder sources — so a bare `simulator` quietly claims the unqualified name and would need a breaking rename the day a second one lands. The `hl7v2:` prefix is free today, groups any future HL7v2 tooling, and echoes the SDK's existing interbox-hl7v2-* skill vocabulary. Domain prefix rather than a location one (`utils:`), which would encode a directory the caller doesn't care about.
Binding to loopback keeps the network out but does nothing about the developer's own browser, and the README claimed otherwise. Bun's req.json() ignores Content-Type, so every mutating route was reachable from any website the developer visited: a plain <form enctype="text/plain"> POSTing a JSON-shaped field name is a CORS simple request — no preflight, no origin check, and the attacker never needs to read the response to have caused the effect. Verified end to end; POST /export with clean:true deleted a target directory's contents cross-origin. - Guard every route: Host must be one we expect (this is what stops DNS rebinding, which loopback binding cannot), and POST must carry application/json. A form cannot send that content-type, and fetch() with it preflights, which this server answers with no CORS headers. The check is applied by wrapping the route table, so a route added later cannot forget it. POST only: PATCH and DELETE are never simple requests, and demanding a body content-type on those would break legitimate bodiless calls. - HOST used `??`, so an empty-but-set HOST (docker run -e HOST, a Kubernetes `value: ""`) reached Bun as hostname:"" and bound every interface — while Bun still reported "localhost", so the log looked fine. Use `||`, log the real bind address, and warn loudly on a wildcard. - Confine /export and /export/stream under EXPORT_ROOT. Resolving and then testing containment is the only check correct on both platforms; rejecting ".." textually misses Windows drive-relative and UNC forms. - Scope `clean` to the .hl7 files we wrote. The blanket rm destroyed unrelated files and, lacking `recursive`, threw on the first subdirectory — deleting whatever sorted ahead of it and then reporting written: 0. - Strip HL7 delimiters from source names and bound the length. A name became MSH-4 by raw interpolation, so `|` shifted every later field along and let a caller forge MSH-9 (what receivers route on) and MSH-10 (what they dedupe on) in traffic aimed at a real engine. - Add ceilings: MAX_SOURCES, MAX_SSE_CLIENTS, MAX_STREAM_FILES, and a 1000-message burst cap. Drop SSE frames when a client stops draining rather than queueing without limit, and release the subscriber slot on disconnect. - Validate persisted sources on load, escape `type` in attribute context, and escape `<` in JSON embedded in <script>. - Return a generic 500 instead of the raw error, which carried absolute paths.
…r the gaps Follow-up to the security pass: the correctness and duplication findings from the same review. Bugs, each reproduced before fixing: - A source could become permanently unstoppable. stop() flips `running` synchronously, but the loop can sit in an uncancellable sleep for a whole inter-arrival gap — seconds at low rates. A start() in that window began a second run; when the first unwound, its tail clobbered the live run's state and stop() then early-returned forever. Runs now carry a token and only the current one may write the tail. Reachable by clicking Stop then Start. - A rejected PATCH left memory, disk and the actor's pacing disagreeing: update() mutated the live definition field-by-field and validated afterwards. Validate everything first, then assign. - Every source emitted identical control IDs, placer/filler numbers and visit numbers, because profileFor specialized only the MRN. A simulator built to exercise routing and dedup was handing the receiver one system replayed. - sendOverMllp abandoned connected sockets when any one failed, and `open` removes its own error listener once connected — so a later error on a leaked socket was an unhandled 'error' event, which is fatal. allSettled plus an explicit teardown. - The reliable path keyed its response map by message TEXT, so two identical bodies shared one entry and the last outcome overwrote the first, losing a refusal it had been told about. Key by index. - writeFrame leaked a `drain` listener per frame on the error path; probePort leaked a socket whenever the timeout won its race; Rng.pick/weighted returned undefined on an empty distribution, which reached the wire as the literal string "undefined". Generator unification: The block "draw a message, roll faultRate, pick a fault, apply it" existed in five places and they had drifted — src/ drew fault decisions from the seeded Rng, ui/ used Math.random(). So "its own seeded RNG" held for `bun run gen` and silently did not for anything the UI drove. One makeGenerator now serves all five, so the seed governs content and faults alike, and /export reports the seed it used so a batch can be reproduced. Also removed genuinely dead code: sendOverMllpStream, streamGenerator, getState. Tests: 49 -> 75. The additions are the ones that earn their place — a golden vector for the PRNG (change its constants and every other test still passes while every generated corpus silently becomes irreproducible), prove-it tests for the clean-deletes-user-files and rejected-patch bugs, the refused-vs-silent split that had no coverage at all, MSH injection, and toRow's column contract. Test hygiene: scratch files go to the OS temp dir and are removed, four un-awaited rejects assertions now await, and listeners close even when a test throws.
ipasechnikov
force-pushed
the
feat/hl7v2-simulator
branch
from
August 5, 2026 17:40
1c69b88 to
6f97556
Compare
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.
Vendors hl7v2-simulator into
utils/hl7v2-simulator/so workspace users can generate synthetic HL7v2 trafficwithout an upstream system to point at.
Addresses HealthSamurai/interbox#93.
How you use it
From the repo root, with the dev stack up:
Installs the simulator and opens its UI on http://localhost:4003, already
pointed at the MLLP port
docker-compose.yamlpublishes. Click Start alland messages land in the dashboard.
Why it is not in the root install
It keeps its own
package.jsonand lockfile instead of joining the rootinstall. A workspace that never simulates traffic shouldn't carry
fakerandfriends on every
bun install. Consequences, both handled:bunfig.tomlscopes rootbun testtotest/. Barebun testwalks thewhole tree and would otherwise collect the simulator's suite, which cannot
resolve its imports from the root — verified to fail 5 tests, so CI would
have broken.
simulatorjob withworking-directoryset, runningbun install --frozen-lockfile+ typecheck + tests for that package.Packaging changes on top of upstream
The vendored tree is otherwise verbatim. The delta:
HOSTto override).Bun.servebinds
0.0.0.0when given no hostname, and this server has noauthentication:
/exportwrites and — withclean: true— deletes files ata path taken straight from the request body, and
/probeopens TCPconnections on request. Acceptable on a dev box; not something to expose to
the network the moment we tell customers to run it. Confirmed with
netstat:was
0.0.0.0+[::], now127.0.0.1only, withHOST=0.0.0.0stillworking as the documented escape hatch.
(new
src/paths.ts). Starting from the repo root previously failed withENOENT on
fixtures/profile.json, and scattereddata/sources.jsonwhereveryou happened to be standing. Paths passed explicitly are still honoured
as-is.
INTERBOXMSH-5/6 overridable viaRECEIVING_APP/RECEIVING_FACILITY, for stands that route on the receiver fields.--hotfrombun run ui(kept asui:dev). The server owns alistener and per-source actors that a hot reload leaks — the same shape that
bit the engine's dev watcher.
typescriptfrom peer to dev deps so the new CI job has atsc.unknown typeerror to list all four source types (pharmacywasmissing).
Docs
utils/hl7v2-simulator/README.mdis the quick start the issue asked for:install, run, point at a target, choose source types — plus fault injection,
the generator CLI, and the HTTP API. Every claim was checked against the code.
Two stale claims corrected while writing it, both pre-existing:
bun run bundle, which is not a script(the engine bundles at boot)
--loadflag that has no handlerVerification
bun install --frozen-lockfileworks against the committed lockfile
ACK-ing listener, run from the repo root
curlexamples work verbatimfixtures/profile.json(163KB) scanned before publishing to a public repo:faker-generated catalogs, anonymized lab systems, public LOINC codes, no PHI,
no internal hostnames, no credentials
Notes for review
+N(currently
1.11.3+0), which can ride this PR or come from the Releasebutton after merge — your call.
guard-versionskips when the version isuntouched.
package.jsonasserts"license": "MIT"— upstream has noLICENSE file and this repo is MIT, so it inherits sensibly, but worth an
explicit nod since this is the moment the code becomes MIT in public.
hl7v2-simulatoris private and untouched by this PR, so theREADME deliberately does not link to it. Worth deciding separately whether it
gets archived or redirected here.
🤖 Generated with Claude Code
Update: review pass
A five-axis review of the vendored code (architecture, security, tests) found
issues serious enough that they are fixed here rather than filed. Each was
reproduced before fixing and verified after.
The headline one invalidates a claim made earlier in this PR. Binding to
loopback does not protect against the developer's own browser: Bun's
req.json()ignores Content-Type, so every mutating route was reachable fromany website they visited, via a plain
<form enctype="text/plain">— a CORSsimple request, no preflight, no origin check. A drive-by page could delete a
directory's contents through
POST /export. Now guarded by aHostallowlist(against DNS rebinding, which loopback binding cannot address) plus an
application/jsonrequirement on POST, applied by wrapping the route table soa new route cannot forget it.
Also fixed:
HOST=""silently bound all interfaces (??where||wasneeded); export directories are confined under
EXPORT_ROOT;cleandeletesonly the
.hl7files the tool wrote — it previously removed unrelated filesand, lacking
recursive, threw on the first subdirectory, so it lost dataand failed; HL7 delimiters in a source name let a caller forge MSH-9 and
MSH-10 on traffic aimed at a real engine.
State-machine bugs, all reproduced: a source could be left permanently
unstoppable by Stop-then-Start; a rejected PATCH left memory, disk and pacing
disagreeing; every source emitted identical control/placer/filler/visit IDs,
which defeats the point of a multi-source simulator; sockets and listeners
leaked on several error paths.
The generate-and-maybe-corrupt block existed in five copies that had drifted —
src/used the seeded RNG,ui/usedMath.random()— so the documentedper-source determinism silently did not hold for anything the UI drove. One
makeGeneratornow serves all five.Tests 49 → 75, chosen for what they catch rather than for coverage: a PRNG
golden vector (its absence meant the constants could change while every test
passed and every generated corpus became irreproducible), prove-it tests for
the two data-integrity bugs, the refused-vs-silent accounting that had no
coverage at all, and MSH injection.
Consciously not done
Pooling is the right fix but is invasive in the most delicate file here. The
acute case is defused by lowering the burst cap from 10 000 to 1000, and the
behaviour is now covered by tests, so the refactor can land on its own.
/classicstill loads Alpine and Geist from CDNs, so it needs internet.The default topology view is self-contained. Vendoring them is a separate,
larger change.
/probeports. With CSRF closed, reaching it requireslocal access, and restricting it would break pointing the simulator at an
arbitrary local listener — which is the normal workflow.