Remediate TransTrack validation findings (C/H/M) - #229
Merged
Conversation
… boundary C-1: A patient-level SMART grant could read, search, update, delete and bulk- export any patient's resources within the organisation. The scope check returned true for reads (no subject available to compare) and explicitly returned true for searches of non-Patient types, deferring to a server-side filter that did not exist. - Add server/src/fhir/compartment.js: the FHIR R4 CompartmentDefinition/patient path map, an in-process membership check, and a parameterised JSONPath SQL predicate. Both the check and the predicate are driven by the same map so they cannot drift. Unknown and non-compartment types fail closed. - scopes.js: replace isAllowed's blanket search allowance with resolveAccess, which reports the granting access level and refuses patient-level grants on types outside the patient compartment. - middleware/auth.js: pin auth.compartment.patient when the grant is patient-level so enforcement cannot be forgotten by a route. - fhir/storage.js: enforce the compartment on read, search, update, softDelete and history, and refuse writes that would place a resource outside it. - fhir/bulkData.js: constrain $export to the launch patient and use the compartment paths rather than a subject/patient best-effort filter. H-4: authorise every transaction-bundle entry with the same scope check the individual CRUD routes use, before executing any entry, and bound bundle size. M-9: native JWTs bypassed FHIR authorisation entirely. Enforce a role matrix so a viewer can no longer create, update or delete FHIR resources. Adds 29 regression tests; 16 of them fail against the previous code. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
C-4: electron/functions/validators.cjs was dead code — referenced only by its own definition and export. Clinical range checking existed solely in the React form, so IPC, REST, FHIR import, the FHIR webhook and HL7 v2 ingestion could all persist clinically impossible values (MELD 250, negative LAS). - Expand the validator into the single clinical-validation authority: MELD-Na, MELD 3.0, PELD, KDPI and EPTS ranges; calendar-date sanity (no future or impossible birth dates); laboratory plausibility bounds in canonical units; and unit-string checking so a umol/L creatinine is rejected rather than scored as mg/dL. Every range names its controlled-source id. - Add validateEntity/assertValidEntity with a per-entity rule table that is safe for partial updates. - Invoke it at entity:create, entity:update, HL7 ingest (insert and demographics update), FHIR import, the FHIR webhook, and the server's patientService create/update. The server shares the same module by relative path, as it already does for the calculators, so the desktop and thin-client tiers cannot enforce different rules. Also registers previously ungrouped suites in the runner and adds an orphan check so a new test file can no longer sit outside every group (H-8 groundwork). Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
M-13 PGSSL=require set rejectUnauthorized:false, which encrypted the
connection to the database while accepting any certificate presented.
Both TLS modes now verify the chain against PGSSL_CA_FILE or the system
trust store; require differs from verify-full only in hostname
checking. Skipping verification needs PGSSL_ALLOW_UNVERIFIED, which is
refused in production.
M-14 With CORS_ALLOWED_ORIGINS empty and NODE_ENV=development, the origin
option was the boolean `true`, so @fastify/cors reflected any
requesting origin alongside credentials:true and a hostile page could
read authenticated responses. The origin is now always matched against
an explicit allowlist: the configured one, else a fixed localhost list
in development and test, else nothing.
M-15 .env.example and docker-compose.yml shipped JWT secrets that met the
32-byte floor while being fully public, plus a fixed Postgres password.
Neither file now carries a usable secret: compose fails to start
without POSTGRES_PASSWORD and JWT_SECRET, .env.example holds a
placeholder too short to parse, and config.js refuses known
placeholders and filler-padded values in production. Every compose port
is also published on loopback only.
M-16 No unhandledRejection or uncaughtException handler was registered, so
either failure mode skipped the shutdown path entirely. Both now log
and drain the server before exiting non-zero.
L-8 /ready returned the driver's error message (host, port, database, role)
to an unauthenticated caller, and the unique-violation handler returned
PostgreSQL's err.detail, which echoes the conflicting values. Both are
logged server-side and answered generically.
Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
…tener
H-3 Migration 008 created hl7_dead_letters and hl7_sending_apps with no
row-level security, and 006 carried a comment claiming issued_licenses
was "protected by row-level security" when no such DDL was ever
written. On top of that, POST /hl7/dead-letters/:id/replay selected by
id alone, so an admin in one organisation could replay another
organisation's quarantined PHI into their own; the sending-app list and
delete routes spanned every tenant the same way.
Migration 010 enables and forces RLS with app_current_org_id() policies
on all three tables, following 003_rls.sql. hl7_sending_apps gets one
SELECT-only carve-out because the MLLP listener resolves MSH-3 to an
org before any tenant context can exist. issued_licenses is written by
the Stripe webhook, which is keyed by subscription and has no org
context, so that path declares itself through app.billing_context via
the new pool.withBillingContext rather than the policy being weakened.
The route queries carry explicit org predicates as well, so a database
with misconfigured RLS is not the only thing standing in the way.
M-27 The listener wrote dead letters with a NULL org_id, so raw PHI
accumulated unowned. A NULL would have been invisible under the new
policy but still unattributable, so migration 010 instead creates a
reserved INACTIVE organisation (the all-zero UUID), re-attributes the
existing NULL rows to it, and makes the column NOT NULL. Nobody is a
member of that organisation, so its rows are readable by no tenant.
Sending-app resolution also now tries the facility-qualified key before
the bare application name.
H-9 MllpFramer.push concatenated indefinitely when no end block arrived — a
remote memory-exhaustion DoS from an unauthenticated peer. The buffer
is capped (1 MiB default), the framer releases it and throws on breach,
and the listener destroys the connection, logging the bound rather than
the bytes. Connections also get an idle timeout and a concurrency cap,
and the listener warns when it is reachable off-host without TLS.
Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
H-12 POST /cds-services/:id wrote the complete CDS Hooks request and
response into cds_service_invocations. A request carries the patient
context plus every prefetched FHIR resource the EHR resolved for us, so
the audit trail had become a second copy of the clinical record with
none of the minimisation or retention rules that govern the primary
store. The route now writes a structured summary — hook, context and
prefetch key names, resource types and counts, card counts and
indicators, sizes, duration, error — and copies no value or free text
out of either payload. Full capture is opt-in via
CDS_CAPTURE_RAW_PAYLOADS; captured rows are flagged and given an
explicit raw_payload_expires_at so the retention expectation lives on
the row rather than in a runbook.
Invocation was also gated by nothing but global authentication, so any
token in the organisation could pull decision-support cards about any
patient the service could resolve. A SMART token now needs a FHIR read
or search scope, and a native JWT needs a role that may read patient
data.
L-15 The feedback endpoint answered { acknowledged: true } and discarded the
body, so every EHR sending outcomes believed we were recording them.
Feedback is persisted to cds_service_feedback (coded override reasons
only, never the clinician's free text), and a failure to store now
surfaces as a failure instead of a false acknowledgement.
Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
…ount
M-10 findUser's unscoped query ended in LIMIT 1 and /auth/login called it
with no orgId, so an email registered in two organisations
authenticated into whichever row the planner happened to return —
possibly a tenant the user has no relationship with. The query now
looks for a second match and refuses the login with
`organization_required`; /auth/login accepts an optional orgId to
disambiguate. The same rule applies to the SMART password flow.
setLockedUntil and isLockedOut both keyed on email alone, so five
failures against one tenant locked that address out of every other
tenant — a cross-tenant denial of service that needed no credentials.
Both now key on the resolved user id, and the failure window counts
only attempts recorded against that user's organisation. The account is
therefore resolved before the lockout check rather than after.
M-27 A session in the reserved quarantine organisation is refused here too,
so a stray user row could not turn unattributable dead-letter PHI into
readable PHI.
M-26 The MFA-enrolment token was signed and verified with no audience, so
only the `purpose` claim separated an enrolment grant from any other
token this issuer signs. It now carries a dedicated
"<JWT_AUDIENCE>:mfa-enroll" audience, and the enrolment routes verify
it.
Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
…cally
M-11 The consent page carried launch_patient as a hidden field and POST
/oauth2/authorize trusted whatever came back, so anyone who could reach
the consent endpoint could name an arbitrary patient and get an
authorization code whose launch context pointed at them. The launch is
resolved once at GET /authorize and stored in smart_launch_contexts;
the form carries only an opaque, hashed, single-use handle bound to the
client that started the launch. A missing, expired, spent or
mismatched handle yields no launch context — never a client-supplied
one.
M-26 makeIdToken signed the SMART/OIDC ID token HS256 with JWT_SECRET, the
same key that signs our own API access tokens. Every relying party
would have needed that secret to verify an ID token, and any client
holding it could mint access tokens for any user in any organisation.
ID tokens are now signed RS256 (or ES256) with a dedicated key and the
public half is published at /.well-known/jwks.json, advertised through
jwks_uri in the SMART configuration. Production refuses to mint a token
without SMART_ID_TOKEN_KEY_FILE, because an ephemeral key differs per
replica and per restart.
L-14 verifyAssertion required a jti but kept no record of the ones it had
accepted, so a captured Backend Services client assertion could be
replayed until its exp. Accepted (client_id, jti) pairs are recorded in
smart_client_assertion_jtis, uniqueness is enforced by the primary key
so concurrent redemptions cannot both win, and the jti is only recorded
after the signature verifies so the cache cannot be poisoned against a
legitimate client. Expired rows are reaped on a throttle.
Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
M-12 Six of the seven calculator routes handed req.body straight to the
scoring function with no schema at all, so unvalidated client input
reached the formulas and any extra property travelled through into the
echoed `inputs` block. Each route now parses an explicit Zod schema
whose required fields mirror the calculator's own REQUIRED_FIELDS, and
unknown keys are stripped.
The one route that did have a schema, /calculators/meld, declared
fields (bilirubin, inr, creatinine) that the calculator does not take —
it destructures creatinine_mg_dl, bilirubin_mg_dl and inr — so the
schema stripped the body down to keys the formula ignored and every
request scored INSUFFICIENT_DATA. The schema now matches the contract.
PATCH /patients/:id used z.object({}).passthrough(), which let any
caller with write access set any allowlisted column by naming it:
priority_score, meld_score, psychological_clearance, waitlist_status
and the rest. The writable columns are now enumerated with their types,
server-owned columns are absent, and a body with nothing writable in it
is rejected rather than sent to the service layer. POST /patients uses
the same allowlist instead of its own passthrough.
Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
H-11: appendAuditRecord in auditChain.cjs is now the only way an audit row is written. It computes the chain fields inside the insert transaction and throws when it cannot, so an operation that cannot be evidenced does not proceed. verifyAuditChain no longer filters rows without record_hash — an unchained row is reported as a missing_hash failure, which is what makes the direct inserts that used to bypass the writer visible instead of invisible. M-6: audit_logs gains a per-org seq column (migration 19, additive) that is part of the canonical signed payload and the chain ordering, so append order no longer depends on a clock a local admin can move. Verification detects gaps, renumbering and non-monotonic timestamps, and treats pre-migration rows as sequence-exempt explicitly rather than skipping them. healthCheck reports the cached startup verification result, so a detected break surfaces as a degraded state rather than only in the logs. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
…lass H-2: the old check called PRAGMA cipher_version and set encryptionEnabled unconditionally — that pragma returns an empty result in this build even on a plaintext database, so it could not fail. verifyDatabaseEncryption now proves five independent things: the cipher profile in force is sqlcipher, kdf_iter is the 256000 the compliance claim quotes, a data page reads back with the configured key, the file carries a SQLCipher salt, and the bytes on disk do not begin with the plaintext SQLite magic. A packaged build that cannot verify closes the handle and refuses to start; getEncryptionStatus reports the evidence rather than a bare boolean. M-7: applyCipherPragmas is the one definition of the profile, used by the normal open path, migrateToEncrypted, backup verification and restore, so those paths cannot drift apart again. M-4: break-glass now refuses on a packaged build unless the operator confirms with the account name, writes a high-severity chained audit record before the credential changes, always forces must_change_password, and only clears MFA when explicitly asked — as a separately audited event. Audit rows written by init.cjs itself now go through the chained writer (auditSystemEvent) so no unchained rows exist (H-11b). Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
… gate PHI exports H-5: logger.write applies phiRedaction to the message and the metadata before the entry reaches any sink, so the disk log, the dev console and the optional remote collector see the same redacted text whether or not the call site remembered. A re-entrancy flag and a fail-safe branch mean redaction can neither recurse nor emit the unredacted value when it throws. L-10: forwarded audit rows carry a stable salted HMAC of the workforce address by default rather than the address itself. TRANSTRACK_SIEM_WORKFORCE_ID selects raw or omit; an unrecognised value falls back to pseudonymous, so a typo cannot start exporting mailboxes. M-8: _writeKey refuses to write a plaintext key file on a packaged build when safeStorage is unavailable, matching init.cjs rather than silently degrading. M-25: a support bundle containing free text requires an explicit confirmation token and a named operator, is marked PHI-bearing in the payload and the filename, and cannot be produced when either is missing. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
H-1: entity:list and entity:filter for Patient now require a valid PHI grant. A list-scope grant (entityId '*') lets a coordinator work a worklist with one justification instead of one per row, and the read stays audited. H-11a: logAudit delegates to the chained writer and no longer degrades to a row without hash-chain fields; a failed audit write fails the originating operation. M-1: labs, barriers, organOffers, livingDonors, postTransplant and operations enforce the accessControl permission model instead of only checking that somebody is logged in. shared.requirePermission/requireAdmin keep the failure mode uniform. Offer transitions that carry a signature require MATCH_APPROVE; administrative ones require MATCH_UPDATE. M-2: function:invoke dispatches only through an allowlist that names the permission each function needs, so importFHIRData and pushToEHR are no longer reachable by any authenticated role. M-3: a packaged build ignores the dev escape hatch entirely, so it cannot be talked into the relaxed CSP by an environment variable. M-5: auth:loginHints returns only isPackaged and setupTokenPresent — not the setup token path, whether an admin exists, or the default admin address. M-22: update check/download/install require an admin session, and download and install refuse outright when electron-updater signature verification is not configured for the build. L-5: restore and backup paths resolve through fs.realpath and must land inside the application data directory, the backup directory, or an explicitly configured export directory. Write targets are additionally limited by extension so a backup cannot be aimed at the encryption key file. L-7: every preload listener strips the IpcRendererEvent and returns an unsubscribe that removes the wrapper actually registered. I-1: the remaining markers in these files are replaced with the constraint they were standing in for. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
… on gaps C-3 / H-10 / L-11. Verification: tests/calculatorReferenceVectors.test.cjs replaces restatements of the implementation's own arithmetic with vectors derived from the OPTN policy text and evaluated longhand — the Rao xB=0 reference donor, each KDRI coefficient in isolation, and the MELD/MELD-Na/MELD 3.0 equations transcribed from OPTN Policy 9.1.D. Defect found and fixed while sourcing: MELD 3.0 applied the adult intercept (6) and the 1.33 female term to every candidate aged 12 and over. OPTN publishes a distinct equation for candidates 12-17 with intercept 7.33 and no sex term. age_years is now required because it selects the equation. PELD: OPTN replaced PELD with PELD-Cr on 2023-07-13. The code implemented the superseded pre-2023 equation. The per-term coefficients exist only in Table 9-1 of OPTN Policy 9.1.E, which is an image in the policy PDF; a secondary source was found but contradicts OPTN's own narrative description. Rather than ship guessed coefficients for a pediatric liver score, calculatePELD now fails closed with REFERENCE_DATA_UNAVAILABLE naming the missing table. The superseded equation survives only as calculatePELDLegacy2016, stamped superseded, for historical reconciliation. Recorded as residual risk RR-01. The PELD albumin floor of 1.0 that the report flagged for reconciliation is confirmed CORRECT against OPTN Policy 9.1.E. H-10: the KDPI median-KDRI scaling factor and both percentile maps move out of code into provenanced reference tables. Every result now names the source revision; an overdue review date flags the result stale, degrades the health check and fails the build. A missing table produces no score rather than a substituted one. C-3 (LAS): the module presented an invented heuristic as the Lung Allocation Score. Renamed to the TransTrack Lung Triage Index (TTLI), flagged isPublishedInstrument: false on every result, and removed from ALL_FORMULAS as 'LAS'. The las_score column still stores a real LAS obtained from UNet. L-11: KDPI rejects a zero donor age instead of extrapolating the Rao age spline. Adds docs/compliance/CLINICAL_SOURCES.md, the controlled source register that finding C-3 said did not exist. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
The plain-Node suites stubbed electron.app.getPath() with a directory inside tests/, so running them wrote SQLite databases, logs and key material into the repository working tree. services.test.cjs never removed its directory at all, and a suite that failed part-way left the others behind too. Adds scripts/test-temp-dir.cjs, which allocates the directory under the OS temp dir and removes it on process exit — which also covers an uncaught exception and an explicit process.exit() — plus SIGINT/SIGTERM/SIGHUP for cancelled CI jobs. phiLeakage.test.cjs pointed userData at tests/ for the same reason and now uses the helper, which is also what lets its redaction tests execute the real logger. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
scripts/production-audit.mjs carried a hardcoded ALLOWED set of GHSA ids and was what CI actually ran; scripts/audit-with-exceptions.mjs read security/vulnerability-exceptions.json and enforced reviewBy expiry, staleness and severity-increase invalidation. Two sources of truth, and the CI path was the one without the expiry check. Removes production-audit.mjs. audit-with-exceptions.mjs gains --scope so the server workspace (which has its own lockfile) is audited by the same gate against the same allowlist file; an exception declares its scope and never silently covers the other workspace. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
release:check reported "RELEASE GATE: PASSED - build is releasable for first-customer pilot" and exited 0 on a tree with no installer and no signing credentials, because the installer/signing/notarization gates were 'optional' unless --for-sale or TRANSTRACK_RELEASE_CHANNEL=public was set. Those gates are now mandatory by default, so the ordinary invocation blocks. --allow-unsigned (or TRANSTRACK_ALLOW_UNSIGNED=1) is the explicit developer escape: it waives them, reports NOT RELEASABLE rather than PASSED, lists the unmet release requirements, and still exits non-zero (3) so no downstream automation can read it as a green release gate. --for-sale refuses the waiver. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
FUNCTIONAL_SUITES listed 'apiClientParity.test.cjs', which is not and has never been in tests/. The runner treats a listed-but-missing suite as a hard error, so every group aborted before executing anything: 'node scripts/run-test-suites.cjs core' exited immediately and 'npm test' with it. H-14's parity coverage belongs with the other renderer tests under Vitest. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
localClient (8.6% lines) and remoteClient (5.3%) carry every PHI read and write in the two deployment modes, and each wrapper is a one-line delegation with nothing to catch a renamed IPC channel or endpoint. The new suites walk every namespace, and pin the properties that are silent when they break: the browser-dev mock refuses to fabricate a backup/notice/bundle receipt, the access token stays out of localStorage, a 401 refreshes exactly once, and clinical config is not written to browser storage in a production build. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
These were excluded from coverage on the grounds that Playwright covered them. It does not: no e2e spec navigates to any of the five. They are now measured, and each has a suite covering the rules a coordinator cannot recover from — MFA cannot be removed without re-authentication, a declined offer must carry an OPTN reason code, a deferred/declined donor must carry a reason and a donation must carry the date the Policy 14 schedule is derived from, an HL7 message is not written to the database before it has been parsed and reviewed, and no post-transplant record can be filed without a selected recipient. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
The compliance directory looked like a validation package and was not one. The Validation Plan's status was "Template - to be ratified", the IQ, OQ and PQ protocols were blanks with "_____" execution fields, the Validation Summary Report was a template, and the only fully worked example was labelled fictional. In parallel docs/VALIDATION_ARTIFACTS.md described a second, older v1.0.0 package with empty results tables. No FMEA existed. A reader seeing an IQ, an OQ and a PQ in a compliance/ directory would reasonably conclude the system had been qualified. It had not. A vendor cannot execute a site qualification, so the remediation is to state precisely what is and is not qualified and make the distinction impossible to miss: - VALIDATION_PLAN.md ratified to v2.0, Approved and in force, with an effective date, approver role titles and scope bound to 1.3.0. Separates vendor release verification from site validation, and designates the server tier early access inside the compliance package rather than only in the README. - executed/IQ_TT-IQ-001.md records what could genuinely be evidenced on Linux/Node 22: dependency install, native module build, lockfile integrity, schema and migration creation, file layout, SBOM tooling. Host-specific steps are marked NOT EXECUTED with the reason and the responsible party. - executed/OQ_TT-OQ-001.md records the automated verification that actually ran: 106 test files, 1507 assertions, no failures. Every case cites a test file that exists. The interactive portion is marked NOT EXECUTED. - executed/PQ_TT-PQ-001.md is marked NOT EXECUTED BY THE VENDOR, states why (no clinical users, no site data, no site environment), and supplies the protocol the deploying organisation runs. - VALIDATION_SUMMARY_REPORT.md is the cover document and says plainly which stages are complete and which are not. - FMEA.md analyses 30 failure modes drawn from this system's behaviour, with severity, occurrence, detection, RPN and required action, cross-referenced to RISK_REGISTER.md. - RESIDUAL_RISK.md carries sixteen formal residual-risk statements, each with affected findings, acceptance rationale, compensating controls, accepting role and closure criteria. - docs/VALIDATION_ARTIFACTS.md is withdrawn and now carries only a superseding notice. Two packages of different vintage is worse than one honest package. compliance/README.md is reindexed around the executed package, withdraws the unsupported AATB claim, and states the server tier's early-access status, which the review noted was absent from the compliance documentation (M-17). Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
… exist (I-7, M-17)
scripts/check-compliance-docs.mjs enforces requirement-to-matrix consistency
but does not verify that the test files the matrix cites exist on disk, so a
citation naming no file reads identically on the page to a real one. Every
path in the Implementation and Verification columns was checked against the
filesystem.
Four verification citations named no file:
tests/auth.test.cjs -> tests/ipc-integration.test.cjs,
tests/compliance.test.cjs
tests/passwordPolicy.test.cjs -> tests/business-logic.test.cjs,
tests/passwordHistory.test.cjs
tests/siem.test.cjs -> tests/siemForwarder.test.cjs,
tests/siemRedaction.test.cjs
tests/livingDonor.test.cjs -> tests/livingDonors.test.cjs
Four implementation paths were also stale:
electron/services/passwordPolicy.cjs -> electron/ipc/shared.cjs,
electron/services/passwordHistory.cjs
electron/services/priorityWeighting.cjs -> the module that implements it
electron/services/livingDonor.cjs -> electron/services/livingDonors.cjs
electron/ipc/handlers/livingDonor.cjs -> electron/ipc/handlers/livingDonors.cjs
TT-R010 (single sign-on) was marked "Not implemented - deferred beyond 1.2.1"
while electron/auth/oidcDesktop.cjs and electron/ipc/handlers/auth.cjs ship
OIDC desktop SSO with mandatory PKCE S256. The row now states what is
implemented (OIDC) and what is not (SAML on the desktop; SAML exists only in
the early-access server tier), and cites tests/oidcDesktop.test.cjs. A
traceability matrix that misstates implementation status is itself a
validation defect.
The gate should check file existence; that change belongs to scripts/, which
this matrix does not own.
Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
… shipped system (M-17)
Twelve places where the documentation described a different system from the
one that ships. Each is corrected against the source.
HIPAA posture. DUE_DILIGENCE.md claimed a "HIPAA-compliant desktop
application" while README.md correctly said the opposite. HIPAA compliance is
a determination an organisation makes about itself, its workforce, its
policies and its physical environment; it cannot be an attribute of software.
DUE_DILIGENCE now matches the README.
Network dependencies. The same document claimed the system "operates entirely
on-premises with no external network dependencies", contradicted by the
Fastify server tier, the optional remote log sink (SENTRY_DSN /
TRANSTRACK_REMOTE_LOG_URL), the SIEM forwarder and GitHub Releases
auto-update. Every egress path is now enumerated with its default state and
what crosses the boundary, in DUE_DILIGENCE.md, README.md and COMPLIANCE.md.
AATB. The claim of design and validation against AATB standards is withdrawn
from DUE_DILIGENCE.md, COMPLIANCE.md, HIPAA_COMPLIANCE_MATRIX.md and
GITHUB_SETUP.md. No AATB control mapping ever existed behind it, and
TransTrack is a solid-organ tool rather than a tissue-bank system. Removing an
unsupported claim is preferable to constructing a mapping to justify it.
package.json still carries an "AATB" keyword and the application UI still
asserts AATB alignment; neither is owned here and both are reported.
Part 11 signatures. PART_11_CONTROL_MAPPING.md stated that TransTrack "does
not implement electronic signatures" while electron/services/
electronicSignature.cjs had been implementing signRecord(). Sections 11.50,
11.70, 11.100 and 11.200 now describe what exists - an application-level
signature record binding signer identity, declared meaning, a SHA-256 payload
hash and a timestamp, immutable at the trigger level and tamper-evident by
recomputation - and say equally plainly what it is not: no key pair, no
certificate, no non-repudiation against the system operator, and no
re-authentication at the point of signing, so 11.200(a)(1)(i) is not literally
met. Recorded as RR-13.
Installers. README.md listed filenames at version 1.0.0 and did not reflect
that electron-builder.enterprise.json produces TransTrack-Enterprise-
${version}. The table now gives both build configurations as patterns and
notes that signing credentials are not yet procured (RR-10).
Recovery objectives. DISASTER_RECOVERY.md stated RPO = 1 hour while
policies/BUSINESS_CONTINUITY_AND_DR.md stated 24 hours. Two authoritative
objectives for one system is itself a defect. Reconciled to <= 24 hours, which
is what the product delivers unaided - electron/services/disasterRecovery.cjs
schedules automated backups at autoBackupIntervalHours: 24, so an hourly RPO
was never achievable from the application alone. The BCDR policy is now
normative for the objectives and the DR document is procedural and defers to
it.
PHI egress and secure delete. README.md claimed no PHI leaves the local system
unless exported; qualified as a default rather than a structural guarantee. It
also presented multi-pass secure delete as a guarantee while
electron/services/secureDelete.cjs documents that it is ineffective on SSD,
copy-on-write and snapshotted volumes. The README now says the same, and
points at full-disk encryption plus cryptographic erase as the effective
control (RR-08).
Calculators. The list advertised "LAS". That module is the TransTrack Lung
Triage Index, an internal expert-set instrument that is neither the OPTN LAS
nor the Composite Allocation Score, flagged isPublishedInstrument: false
(RR-07). The README and COMPLIANCE.md now say so, record that PELD is
unavailable pending a verifiable OPTN Table 9-1 source (RR-01), and flag the
KDPI and EPTS percentile maps as approximations (RR-03).
Server tier maturity. Its early-access status appeared in the README but not
in the compliance documentation. It is now stated in the README's validation
status section as well.
Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
…d escalation path (L-13, M-17) The sole security-disclosure and support contact across the product was a consumer webmail address. A vulnerability reporter had no role-based endpoint, no stated response time, and no route past an unresponsive individual. SECURITY.md now defines: - security@transtrack.example for vulnerability disclosure and suspected PHI incidents, and support@transtrack.example for non-security support, both described as group addresses delivered to a role plus a deputy rather than to one person; - response service levels per stage - acknowledgement in 2 business days, triage in 5, status updates every 10, and fix or documented mitigation in 7 / 30 / 90 days by severity, with severity assigned on CVSS v3.1 adjusted upward where PHI confidentiality, audit-trail integrity or a clinical calculation is affected; - a four-step escalation path from Information Security Officer through Engineering Lead and Quality Assurance Officer to Privacy Officer, with 5 business days at each step, and a note that a covered entity's own 60-day breach-notification deadline is not displaced by any vendor timeline; - coordinated disclosure terms. The addresses are placeholders on the reserved .example domain and are not yet provisioned. That is stated wherever they appear rather than implied, and provisioning a monitored role address with an on-call rotation behind it is recorded as a commercial-release prerequisite in RR-15. Until then reporters are directed to GitHub private vulnerability reporting. Also in SECURITY.md: the supported-version matrix listed only 1.0.x while the product shipped 1.2.1 (M-17); it now covers 1.3.x current, 1.2.x maintenance and the end-of-life lines. The AATB conformance claim is withdrawn (M-17). A network-egress section enumerates the five optional egress paths with their defaults and the tests that verify redaction. The "LAS 0-100" range in the threat table is corrected. The remaining contact points in the docs tree are updated to the role-based addresses. LICENSE, TRADEMARK.md, CODE_OF_CONDUCT.md, .github/ SECURITY_ADVISORY_2026-05-08.md, marketing/ and src/components/ ErrorBoundary.jsx still carry the webmail address and are not owned here. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
It was a Docker Compose smoke-test procedure for the server tier. The desktop procedures a regulated deployment actually depends on - backup, restore, key rotation, breach notification, DR drills - lived in other documents and were not reachable from it, so an operator following the runbook would never find them. The runbook now: - indexes every operational procedure against its controlling document, and states which document is normative where two cover the same ground; - gives an operating cadence table naming the evidence each recurring control requires, on the principle that an activity without a record does not satisfy the control; - documents the startup health checks (encryption verification, audit-chain verification, migration status, licence) and treats each failure as a stop condition, with chain-verification failure routed to incident response rather than to routine triage; - carries a disaster recovery drill procedure and log template. The procedure deliberately requires the most recent routine backup rather than one made for the drill, and requires recovering the key from key backup rather than from the production host, because a drill that skips either tests nothing about the path that will be used in a real recovery; - ends with a known-limitations table linking each operational constraint to its residual-risk entry, so an operator meets them here rather than during an incident. The drill log is empty and says why. The BCDR policy mandates quarterly restore drills and none has been executed against this release by the vendor or by any site, so the recovery time objective is a design target rather than a demonstrated capability. Recorded as RR-11 and made a precondition of PQ. The original Docker smoke test is retained as section 7, marked as an evaluation rather than a production procedure, with a note that a passing smoke test exercises reachability rather than correctness against requirements and is not qualification evidence. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
sample-data/ and demo-evidence/ contain synthetic and Epic-sandbox records. They are appropriately fictional, but nothing in the repository recorded that, leaving a reviewer to infer it from the data. Inference is not evidence. docs/TEST_DATA_PROVENANCE.md inventories each location - the 43-entry FHIR bundle authored for this project, the Epic sandbox round-trip transcript whose subject is Epic's publicly documented test patient, the fictional pilot-site example, the in-code test fixtures, and the OPTN coefficient tables - and states for each where it came from and that it contains no real PHI. It also explains why Epic sandbox records are not PHI (fabricated by Epic, published for integration testing, no BAA required) and where that stops: a transcript captured against a customer's live Epic instance would contain PHI, and the FHIR base URL is the gate. Contributor rules follow, including that manual redaction of a production transcript is not an acceptable substitute for regenerating it against the sandbox, and that a fixture directory absent from the inventory has no evidenced provenance. The document is explicit that phiLeakage and loggerRedaction constrain the running application and cannot prove the absence of real PHI in a committed fixture; that assurance rests on the inventory and the contributor rules. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
…L-12) The regulated product repository tracked docs/STRATEGIC_FIT.md, an M&A and partnership positioning brief naming a prospective acquirer, and docs/legal/COMMERCIALIZATION_CHECKLIST.md, a commercialisation plan with indicative pricing, named vendor shortlists and sales outreach templates. Both are deleted. The reason is not that the material was wrong or secret. A regulated product repository is a controlled-document set: everything in it is potentially in scope for a validation review, an audit or a discovery request, and every file in it carries an implicit claim to be current and controlled. Commercial planning documents change on a sales cadence rather than a release cadence, are owned outside engineering, and are governed by no change-control procedure. Keeping them here mixes two document sets with different owners and different audiences, and invites an auditor to read a pricing sheet as though it were a controlled specification. docs/legal/README.md indexes the genuinely product-relevant legal documents - LICENSE, LEGAL_NOTICE.md, TRADEMARK.md, the BAA material, LICENSING.md - and records the removal and the rule for future additions: a document belongs here only if a deploying organisation, an auditor or a regulator would need it to install, operate, validate or lawfully use the software. Nothing product-relevant was lost. The items that mattered - code-signing credentials not yet procured, no third-party penetration test performed, no provisioned vendor domain - are now formal residual risks (RR-09, RR-10, RR-15) with named owners and closure criteria, which is stronger than a progress note in a checklist. References in HECVAT_PREFILL.md and PILOT_DEPLOYMENT_RUNBOOK.md are redirected. CRITICAL_ACTIONS_REQUIRED.md at the repository root now contains a dangling link to the deleted checklist and is not owned here; it is reported for the owning agent to remove. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
There was no CODEOWNERS file and no documented branch protection or mandatory review, so nothing in the repository established that a change to a security or clinical control had been seen by anyone qualified to assess it. .github/CODEOWNERS assigns mandatory reviewers to the paths where a defect is not recoverable by a later patch: the Electron IPC boundary and preload, the database schema and migrations, authentication and SSO, the audit chain and HMAC key handling, encryption and secure delete, the logger and SIEM forwarder, the clinical calculators and their reference data, the server-tier FHIR, SMART and auth layers, the controlled documents under docs/compliance/, the release and signing scripts, and the test suites that verify all of the above. Team handles are placeholders and the file says so, because an unresolvable handle matches nobody and the protection rule then passes silently. CONTRIBUTING.md documents the required branch-protection configuration for main - two approvals on security- and clinically-owned paths and one elsewhere, stale-approval dismissal, code-owner review, conversation resolution, signed commits, linear history, administrators included, no force push - and lists the ten required status checks by their workflow job names. This is recorded in the repository because a protection rule that exists only in a GitHub setting is invisible to a validation reviewer and is lost on a fork or migration. Deviations are change-control exceptions under the change management SOP. The document states honestly that whether these settings are currently applied cannot be evidenced from within the repository, and gives the gh api command to confirm the live rule. The compliance section is also expanded: no PHI in fixtures with a pointer to the provenance document, no second audit write path, no unauthorised route without an explicit authorisation check, no calculator constant change without a controlled source, and a requirement to fix documentation in the same pull request that makes it inaccurate - which is the failure mode finding M-17 recorded. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
Adds component tests for src/lib/AuthContext.jsx and src/components/session/IdleTimeoutManager.jsx, both previously at 0%. Testing the idle manager with a controlled clock surfaced a real defect: handleActivity depends on showWarning and the listener effect depends on handleActivity, so setting showWarning re-runs the effect, which calls resetTimers() and clears the warning and re-arms the 15-minute logoff. The dialog therefore mounts for a single commit and the automatic logoff never fires. Measured with real timers against a 400ms/250ms policy: logout is still uncalled after nearly three idle periods. The fix is in the component, which this change does not own, so the required behaviour is pinned with it.fails and documented in the test file. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
Secret scanning: adds scripts/scan-secrets.mjs, a dependency-free scanner with
14 rules, an allowlist for the eight historical exposures in git history, and a
--self-test mode that fails if any rule has stopped matching its own sample. The
.gitignore has referenced gitleaks-report.json for months with no workflow to
produce it; a committed scanner cannot silently no-op the way a missing action
can. Wired into security.yml on push and PR (working tree) and weekly (full
history), with a commit status that treats anything other than success as
failure.
Removes the escapes that made the other gates decorative:
- Snyk no longer runs with continue-on-error, and the status job no longer
maps a skipped scan to success. With no SNYK_TOKEN the job runs the
committed audit gate at the same high+ threshold over both workspaces, so a
green snyk status always means a scan ran.
- The server audit runs scripts/audit-with-exceptions.mjs --scope=server
instead of npm audit ... || true.
- The lockfile job fails when either lockfile is missing instead of emitting
a warning, and verifies the server clean install too.
- The load-test job runs the suite runner's performance group.
Dependabot: open-pull-requests-limit was 0 in all three ecosystems, which
disables it including security updates. Re-enabled with limits of 5/5/3, minor
and patch grouped into one PR per ecosystem, security updates in their own
group, majors and the native/Electron stack still excluded from automation.
Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
…anels Adds component tests for the two post-login blocking screens (ForceMfaEnrollment, ForcePasswordChange), the CSV roster import in pages/Patients.jsx, and the lab and readiness-barrier panels with their status badges. All were at 0% coverage except Patients.jsx, which was at 23% with the whole import path unmeasured. The import path is the highest-risk code among these: it writes patient records in bulk from a user-chosen file and decides row by row what to skip. The tests pin that every rejected row is reported with its file row number, that a non-numeric MELD is never coerced into a record, that a partial import is reported as partial, and that a stale success banner cannot survive into the next attempt. Recorded while writing these: the delete confirmation dialog in ReadinessBarrierList is unreachable (nothing calls setDeleteConfirm), and the three password fields in ForcePasswordChange have labels with no htmlFor, so they are not programmatically associated for a screen reader. Both are noted in the test files. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
The gate was inline in ci.yml with hardMin = 19 for lines and branches; the 60%
target only emitted a ::warning::, which cannot fail a build. Five IPC-bound PHI
pages were also excluded from measurement, so 19% was measured against a
flattering denominator.
- Floors now live in scripts/coverage-floor.json and are read by both
vite.config.js (as Vitest thresholds, so a local --coverage run enforces
what CI enforces) and scripts/coverage-gate.mjs. Enforced: lines 57,
statements 57, functions 59, branches 43, plus per-file floors on 22 files
covering every PHI-handling screen and the whole data-access layer.
- The gate script adds a ratchet: coverage more than 8 points above a floor
fails with an instruction to raise it, so the floors cannot drift behind
reality and can only move up.
- ci.yml now runs the runner's "all" group (every Node suite, including the
ones previously reachable only through bespoke npm scripts) instead of
"npm test", whose pretest/posttest hooks rebuild the native module for
Electron and leave the wrong ABI in place for the steps that follow.
- Replaces the deleted scripts/production-audit.mjs reference with the single
exception-checked audit gate, drops the "|| true" from the server audit and
continue-on-error from the packaged-native verification (M-18, M-19).
- Adds a ci-required aggregate job so adding a job to this workflow blocks a
merge without also editing the branch protection rule.
Measured: lines 19.35% to 58.63%, branches 20.31% to 44.54%, with the five PHI
pages back in the denominator.
Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
…repping for them The encryption check in this suite was already converted to a runtime test. The three checks left behind read electron/ipc/shared.cjs and asserted that the strings 'minLength: 12', 'SESSION_DURATION_MS' and 'MAX_LOGIN_ATTEMPTS' appear in it, which a commented-out constant or one that is defined and never consulted also satisfies. They now call the exported validatePasswordStrength across five rejection cases plus an empty credential, and read the exported SESSION_DURATION_MS, IDLE_TIMEOUT_MS, MAX_LOGIN_ATTEMPTS and LOCKOUT_DURATION_MS, bounding each against the policy the compliance matrix claims. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
The snyk job carried "if: github.event_name != 'schedule'", which skipped it on the weekly run — the one run whose purpose is to catch an advisory published against code that has not changed. With the status job no longer treating a skipped scan as a passing one, leaving the condition in place would also have reported a failure every Monday. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
retries and workers were configured but forbidOnly was not, so a `test.only` left in a spec would reduce the Playwright job to a single test and still exit 0. That is the same failure shape as the soft assertions in this finding, one level up: a green check that covers almost nothing. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
…evel (H-4, M-28) The compartment suite covers requireSmartScope and the storage guards in isolation, but nothing exercised POST /fhir itself, so the properties that only exist at the route level were unverified: that every entry is authorised before any entry executes, that a mixed allowed/denied bundle commits nothing, and that the entry cap and method/type validation hold. 16 tests through a real Fastify instance. 9 of them fail against the pre-remediation route. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
- Mount BulkPhiAccessGate for list/filter PHI grant prompts (H-1). - Purge PHI caches and session storage on logout (H-13). - Redact patient-id path segments in NavigationTracker (H-13). - Fix IdleTimeoutManager so warning/auto-logoff timers actually fire. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
- Restore featureGate/tiers and wire session entitlement + write gates (H-6). - Refuse packaged builds that still embed the DEV publisher key (H-7). - Persist trial high-water clock so delete/rollback cannot reset trial (M-21). - Fail loudly for remote functions.invoke and unsupported entities (H-14). - Render clinical dates in UTC with an explicit marker (M-24). - Scrub unsupported AATB product claims; add finding remediation map. Co-authored-by: NeuroKoder3 <NeuroKoder3@users.noreply.github.com>
Unblocks Desktop Build and Security Scanning CI that failed on eqeqeq. Co-authored-by: Cursor <cursoragent@cursor.com>
Restricted sessions reject a second auth:login; complete password/MFA setup on the existing session instead. Co-authored-by: Cursor <cursoragent@cursor.com>
Ruleset expects context 'build' but the job displays as Desktop Build & Tests; mirror the audit/snyk status reporter. Co-authored-by: Cursor <cursoragent@cursor.com>
NeuroKoder3
marked this pull request as ready for review
August 3, 2026 00:44
|
Strix is installed on this repository, but we couldn't run this PR security review because this workspace's trial has ended. Add a card to resume code reviews here. |
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
2 participants
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
Closes the Critical/High findings from the TransTrack Validation Report and the remaining High/Medium items that were still open on this branch (licensing stubs, publisher-key gate, API client silent no-ops, trial reset, UTC date display), with a finding→change map under
docs/compliance/VALIDATION_FINDING_REMEDIATION_MAP.md.Critical
High (this turn completes the last open Highs)
featureGate/tiers; session entitlement + entity write gates--for-salerelease gatefunctions.invoke/ unsupported entities fail loudly; Vitest parity suiteMedium / docs
Test plan
node scripts/run-test-suites.cjs all— 63/63cd server && npx vitest run --config vitest.config.mjs— 328 passednode tests/license.test.cjs— 26 passed (H-6/H-7/M-21)Residual risk
See
docs/compliance/RESIDUAL_RISK.md(PELD RR-01, IRE unvalidated, KDPI/EPTS approximation, RLS not live-Postgres-proven, PQ site-only). Low/informational items (L-1 typing, L-2 module size, I-2–I-4 IRE disclosure) remain partially open and are documented there.