# Technical inspection audit — ALVA **ALVA-DOC-0003 · Broad system audit · Internal engineering review** > **Framing.** This review was performed at the request of the product owner, > applying the lens a technical inspection body (TÜV named as the reference) > would use for a system that produces records intended to be relied on by third > parties. It is an **internal engineering review authored by the development > team** — not an audit conducted by, commissioned by, or endorsed by TÜV > Rheinland, TÜV SÜD, TÜV NORD or any other inspection body, and it carries no > external standing. No certification is claimed or implied. > > Standards are named below where a specific clause frames a finding. Naming a > standard is not a claim of conformity to it. > > **Scope.** The whole system as of `3b83fc9`: the React client, the platform > service (`services/plattform`), the AI orchestrator (`services/ai-orkester`), > the shared domain modules, the database schema, the CI pipeline and the > operational scripts. > > **Basis.** Source code and the running system. Every finding below was > reproduced against a real Postgres and a real server process, or against the > shared modules. **Nothing is reported that could not be reproduced.** Where a > reproduction is shown, it was executed, not imagined. --- ## Summary The two previous revisions examined whether the product's central claim held at the API boundary. It does. This audit asked a different question, the one an inspection body actually asks: > **If a record produced by this system were relied on in a dispute, would it > survive examination?** The answer is a qualified no, for three reasons — and none of them is a defect in the mechanisms the previous revisions hardened. The append-only log is genuinely append-only. The quality gate is genuinely server-authoritative. The event schema is genuinely closed. Those hold. The three findings that decide this audit are about **what those mechanisms do not cover**: 1. **A record can be prevented from ever existing** — by a different tenant, silently, with a `200` response. Append-only protects what is written; it says nothing about what is stopped from being written. 2. **The measurement chain is self-asserted.** The instrument register exists, is well designed, and is never consulted. A client can claim any calibration date for any instrument and the system will grade the result E4 — its highest evidence class. 3. **Erasure is not durable.** The key that crypto-shredding destroys is stored in the same database as the ciphertext, and the restore test explicitly asserts that keys survive a restore. A backup taken before an erasure restores the erased personal data in readable form. Each is a case of a control that is correct in its own terms and incomplete at its boundary. That is the characteristic failure of a system that has been hardened finding-by-finding — which is exactly what the previous two revisions did, and did well. | Severity | Count | Meaning | | --- | --- | --- | | Critical | 3 | Blocks any use where the record may be relied on by a third party. | | Major | 4 | Blocks series deployment. Closable within one release cycle. | | Minor | 5 | Track and schedule. | --- ## Critical findings ### T-1 · One organisation can silently prevent another from recording history `app/src/felsokning/domain.ts:251` · `services/plattform/server.mjs` (event write) · `infra/postgres-init.sql` (`felsokning_handelser`) Three facts combine: | Fact | Where | | --- | --- | | Event ids are generated by the **client** as `Date.now().toString(36) + "-" + counter` — fully predictable | `domain.ts:251` | | `felsokning_handelser.id` is a **global** primary key, not scoped per case or per organisation | schema | | Events are written with `on conflict (id) do nothing` | server | An organisation can therefore occupy identifiers that another organisation will later use — by writing them to **its own** cases, which it is fully entitled to do. When the victim's client syncs an event with a colliding id, the insert is discarded and the API answers `200 OK`. **Reproduced** against a real server and database: ``` org B writes 40 events to its OWN case, with ids derived from (now + 60 s).toString(36) + "-" + n ← the victim's future ids org A then documents a fault cause and syncs it: org A received status code: 200 events in org A's case: 0 id 'mshhkoq9-0' belongs to: arende-b CONFIRMED: org A's fault cause vanished. The server answered 200. ``` No request in the attack touches organisation A. The tenant boundary is never crossed in the sense the previous audits tested — and it does not need to be, because the collision happens in a namespace that is shared by design. **Why this is the most serious finding in the audit.** The product's entire proposition is that a closed case is a complete and durable record. Every control built so far protects records **against modification**. None protects them **against non-existence**. A technician sees the event on screen, the sync reports success, the case closes, and the fault cause is simply not in the log. The failure is silent at every layer that could have reported it. The collision *is* counted — `res.spann.spår.kollisioner` (raised as m-2 in Revision 1) — so the signal exists in telemetry. It is not surfaced to the caller, and a counter in a metrics backend is not a control. **Required.** 1. Scope the primary key to the case (`primary key (arende_id, id)`), so an id in one case cannot collide with an id in another. This removes the shared namespace entirely and is the structural fix. 2. Answer `409` on a collision whose stored content differs from the submitted content. A genuine idempotent replay has identical content; a collision with different content is either an attack or a client bug, and both must be loud. 3. Only then consider unguessable ids. Unpredictability alone would reduce the attack to guessing — it would not close it. *Frame: ISO 9001 §7.5.3 control of documented information — protection includes protection against loss of integrity, not only against unauthorised alteration.* --- ### T-2 · The measurement chain is self-asserted; the instrument register is never consulted `app/src/felsokning/ecm.ts:104` · `services/gemensam/handelser.mjs:98` · `services/plattform/server.mjs` (`/api/matdon`) The system grades evidence E1–E5. A measurement is graded **E4** — the highest class available to a technician's own work — when the event carries an instrument id and a calibration date that covers the time of measurement: ```js const spårbar = Boolean(h.matdonId) && kalibreradVid(h.matdonKalibreradTill, post.tidpunkt); lagg(post, "mätvärde", spårbar ? "E4" : "E1", …); ``` Both fields arrive **from the client**, in the event body. `matdonId` and `matdonKalibreradTill` are declared as optional free fields on `matvarde` (`VALFRIA_FÄLT`), and the server never checks either against the `matdon` register — not that the instrument exists, not that it belongs to the organisation, not that the stated calibration date matches the register. **Reproduced.** An organisation with **no instruments registered at all** submits a measurement: ``` instruments registered in org A: 0 stored calibration: 2099-12-31 stored matdonId: finns-inte-i-registret CONFIRMED: the calibration claim is stored untested against the register. ``` The evidence then reports E4, with the instrument designation the client chose to send. **Why this matters more than it looks.** The instrument register is not missing — it exists, it stores serial numbers and calibration dates, it computes validity in SQL (`kalibrerad_till >= current_date`), and the code comment above it states the principle correctly: *"Utan kalibrerat instrument är det inte lägre evidens utan ingen evidens alls."* The register was built for exactly this check. The check was never wired to it. This is the same defect class the product has now closed three times in other places — provenance decided by the caller rather than by the system (Rev 1 C-2, Rev 2 C-5). Here it survives on the one claim where an inspection body would look first. **Required.** On `matvarde`, treat `matdonId` as a reference: resolve it against `matdon` for the caller's organisation, and derive `matdonBeteckning` and `matdonKalibreradTill` from the register rather than accepting them. Reject an unknown instrument. An expired instrument should not be rejected — the measurement was still taken — but the grade must fall to E1 on the *register's* evidence, not the client's. *Frame: ISO 10012 / ISO 17025 §6.4 — a measurement result is traceable only if the measuring equipment and its calibration status are demonstrable.* --- ### T-3 · Erasure is not durable: the shredding key is stored beside the ciphertext `infra/postgres-init.sql` (`personnycklar`) · `services/plattform/aterstallningstest.sh:88` Crypto-shredding is the product's answer to reconciling an append-only log with Article 17. The design is right: identifying fields are encrypted with a key per subject, and erasure destroys the key rather than the record. The key is stored as `nyckel bytea`, in plaintext, **in the same database, in the same cluster, covered by the same backups** as the ciphertext it protects. Erasure therefore reduces to deleting a row next to the data it was supposed to make unreachable. The system's own restore test states the consequence without recognising it: ```bash kontroll "personnyckeln följde med" "1" "$(kor $MAL 'select count(*) from personnycklar')" ``` The test is **correct for disaster recovery** — a restore that loses the keys leaves the log unreadable. But it is the same fact, read the other way: any backup taken before an erasure contains both the key and the ciphertext, so restoring it returns the erased personal data in readable form. Nobody reconciled the two claims, because they live in different documents and are each individually right. **Required.** Keys must not share a failure domain, an access boundary or a backup lineage with the data they protect. The realistic options, in order of preference for a self-hosted AWS deployment: 1. **Envelope encryption with KMS.** Per-subject data keys wrapped by a KMS CMK; the database stores only wrapped keys. Erasure = schedule deletion of the wrapping key or delete the wrapped key with the CMK unavailable to restore. A restored backup yields wrapped keys that no longer unwrap. 2. A separate key store with an independent backup policy and a documented, tested retention window shorter than the shredding guarantee. Whichever is chosen, **the backup retention window becomes part of the erasure promise** and must be stated in the record given to the data subject. Today the record says the subject was erased. For any data covered by an existing backup, that statement is not yet true. *Frame: GDPR Art. 17(1) together with Art. 5(1)(f); ISO 27001 A.8.24 use of cryptography — key management is the control, not the algorithm.* --- ## Major findings ### T-4 · Retention is recorded and never executed `gallras_efter` is computed at close from the case type and written to `felsokning_arenden`. Searching the entire repository, **nothing ever reads it** — no job, no scheduled task, no query, no CI step. Retention is a recorded intention, not an executed control. This is compounded by C-7 from the previous cycle: the column could not even be *written* for an unknown period, because the append-only trigger forbade the update. That defect survived undetected for one reason — nothing downstream consumed the value, so nothing noticed it was missing. **Required.** A scheduled task that destroys the keys of subjects whose cases have all passed `gallras_efter`, that logs what it destroyed to `raderingar`, and that is exercised by a test. Until then the storage-limitation claim should not be made in customer-facing material. *Frame: GDPR Art. 5(1)(e) storage limitation.* ### T-5 · AI-derived measurements enter the log without provenance `app/src/pages/felsokning/ArendeSida.tsx:1735` An instrument photograph interpreted by the model becomes ordinary `matvarde` events: ```js skicka({ typ: "foto", beskrivning: `Instrumentavläsning (…)`, dataUrl: avlasning.foto }); for (const v of avlasning.tolkning.varden) { skicka({ typ: "matvarde", beskrivning: v.beskrivning, varde: v.varde, enhet: v.enhet }); } ``` Two things are done right and should be preserved: the original photograph is kept as evidence alongside the structured values, and the technician must confirm before anything is written. But no `kalla` is set — and `kalla` exists, is declared on `matvarde`, and is used by the protocol-ingest path for precisely this purpose. Once written, a value read by a model is indistinguishable from one the technician read off the instrument. Human confirmation mitigates the accuracy risk. It does not restore the traceability: the record cannot answer *how this number was obtained*, and that is the question an assessor asks about a transcribed reading. **Required.** Set `kalla` on every event derived from a model reading, and show it in the report. The product's own doctrine — provenance decided by the system, not the caller — is already implemented on the protocol path; apply it here. ### T-6 · Password hashing at bcrypt cost 6 `services/plattform/server.mjs` — `crypt($3, gen_salt('bf'))`. `gen_salt('bf')` without a cost argument uses pgcrypto's default. Verified empirically against the running extension: ``` select gen_salt('bf') → $2a$06$DBB3F2JRNlQ/D7DczWF9gu ``` Cost **6**: 2⁶ = 64 iterations, against current guidance of ≥10 (2¹⁰). That is 16× below the floor, and 64× below a common current setting of 12. A second, quieter issue in the same line: hashing in the database means the **plaintext password is transmitted to the database server as a bind parameter**. On a cluster with `log_statement = 'all'` or with parameter logging, or in a query trace captured for debugging, plaintext credentials reach a log surface that is not treated as a credential store. **Required.** `gen_salt('bf', 12)` as an immediate step, with a rehash-on-login migration for existing hashes. Longer term, hash in the application (scrypt or Argon2id via `node:crypto`) so the plaintext never leaves the service. *Frame: ISO 27001 A.5.17 authentication information.* ### T-7 · The data-protection records are the only records without append-only protection Four tables carry append-only triggers: `felsokning_handelser`, `felsokning_arenden`, `fakturor`, `fakturahandelser`. These are exactly the right ones — and the omissions are conspicuous: | Table | What it holds | Protected | | --- | --- | --- | | `atkomstlogg` | Who read which case, and via which share link | **No** | | `raderingar` | The record that an erasure was performed | **No** | | `personnycklar` | Key state, including `radering_begard` | **No** | The access log is the evidence that access control worked. It is the first artefact an assessor asks for after an incident, and it is the one artefact that can be rewritten by anyone who reaches the database — including the account that would have a motive to. The erasure record has the same property: the proof that a subject's data was destroyed can itself be deleted. **Required.** `atkomstlogg` and `raderingar` must be append-only by the same mechanism as the event log. `personnycklar` legitimately needs deletion (that is what erasure *is*), so it needs a narrower rule: `radering_begard` and `nyckel` may change, identity and ownership may not. --- ## Minor findings | # | Finding | Note | | --- | --- | --- | | **T-8** | `/api/atkomstlogg` is implemented but absent from `openapi.yaml`. | The spec is served live and treated as the versioned interface artefact. A data-protection endpoint missing from it is a documentation-conformity gap, and it is the *only* one — a systematic diff of implemented routes against the spec found no other. | | **T-9** | The ECM rule package runs **unsigned by default**. | Handling is otherwise exemplary: an *invalid* signature blocks case closure outright. But with `ECM_REGLER_NYCKEL`/`ECM_REGLER_SIGNATUR` unset, the package is used and merely logged as unsigned. A deployment that never configures signing gets no integrity control and no failure. | | **T-10** | CORS defaults to `Access-Control-Allow-Origin: *` when `TILLATNA_URSPRUNG` is unset. | Low exploitability — tokens live in `localStorage`, not cookies, so a cross-origin page cannot obtain one. Still a fail-open default. | | **T-11** | A JWT without `exp` never expires. | `verifieraJwt` checks `exp` only when present. Every token minted today carries one (single call site), so this is defence-in-depth, not a live defect. | | **T-12** | `react-router` 6.x — two moderate advisories (open redirect via backslash; constructor injection in SSR hydration). | `npm audit fix` available. The SSR issue does not apply to this build; the open redirect does. | --- ## Confirmed strengths An audit that lists only findings misrepresents the system. These were tested and hold. **Provenance of time and identity is server-owned.** `tillPost()` overwrites `anvandare` and `tidpunkt` from the verified session and the server clock, and preserves the client's clock separately as `registrerad_tidpunkt` for offline work. The comment states the reasoning exactly: *"Sätts de av anroparen är loggen en signerad behållare"* för anroparens påståenden. Correct, and rare. **The event schema is genuinely closed.** `granskaHändelse` iterates the event's own keys and rejects anything undeclared, hard rather than by silent stripping. Verified against real traffic. **Append-only is enforced in the database, not only in the service.** Direct `UPDATE`/`DELETE` against the log is refused regardless of which account connects. Verified against a real Postgres. **The quality gate is server-authoritative and single-sourced.** The client no longer restates the rule; it asks the gate and renders the gate's own obstacles. **Invoice numbering is gapless by construction.** A sequence was rejected because it leaves holes on rollback. An exclusive lock is the correct trade for a document series, and the reasoning is recorded in the code. **Restore is tested, not assumed.** `aterstallningstest.sh` restores a dump and verifies that the triggers came back — the thing that is actually lost in a restore. (Its interaction with erasure is T-3; the test itself is right.) **The commercial boundary is modelled honestly.** A customer administrator can neither issue their own invoice nor record it as paid, and without the issuer key nothing can be issued at all. Invoicing fails closed. **The portal is closed, or says it is a demonstration.** Verified by driving the built application, not by asserting on source. --- ## What this audit changes about how the system should be tested Every one of the three critical findings is invisible to a passing test suite, and each is invisible for a different reason. That pattern is the real output of this audit. | Finding | Why the suite could not see it | | --- | --- | | T-1 | It requires **two tenants acting in sequence**. Every existing test drives one organisation, or two in isolation. Nothing models one tenant's writes as an input to another's failure. | | T-2 | The client sends fields; the server stores them; both are internally consistent. **Nothing in the system disagrees**, so nothing fails. The defect is an absent cross-check, and absence has no test unless someone writes the assertion. | | T-3 | It spans the schema, the erasure path and the backup script — **three artefacts that are each correct**. The contradiction exists only when all three are read together, which no test does. | The suggested addition is one kind of test the project does not yet have: an **adversarial tenant** harness, where a second organisation is scripted to interfere and the assertion is that the first organisation's record is complete. T-1 would have been caught on the first run. --- ## Recommended sequence 1. **T-1** — scope the event primary key to the case, and answer `409` on a content-differing collision. Structural, small, and it closes the finding that most directly contradicts the product's central claim. 2. **T-2** — resolve `matdonId` against the register and derive the calibration status from it. Without this, the E4 grade should not be shown at all. 3. **T-7** — append-only on `atkomstlogg` and `raderingar`. One migration. 4. **T-6** — bcrypt cost 12 with rehash on login. 5. **T-5**, **T-4** — provenance on model-derived events; a retention job. 6. **T-3** — envelope encryption via KMS. Largest change, and the one that should not be rushed: it touches key management, backups and the wording of the erasure record given to data subjects. 7. **T-8** … **T-12** — schedule normally. --- ## Closing assessment The mechanisms this system has built are, in this reviewer's experience, ahead of the segment: a server-authoritative quality gate, a closed event schema, database-enforced append-only, server-owned provenance, a closing statement that demands a stated *why*, and a deliberate refusal to derive commercial or clinical authority the system has not earned. Nothing in this audit contradicts that judgement. What the audit found is that the system is strong **inside** each control and untested **between** them. A record cannot be altered — but it can be prevented. A measurement is graded on calibration — which is never verified. A key is destroyed — and restored from backup. In each case the control is right and its boundary is unexamined. That is a good position to be in. Boundary defects are cheap to fix once named, and three of the four Major findings are a single migration or a single query each. But the three Critical findings must be closed before any deployment where the record may be relied on by a third party — an insurer, a court, an OEM warranty function — because in each case the record would not survive the examination it exists to survive. Applying the product's own standard: the erasure guarantee moves from `validated` back to `tested`, and the measurement-traceability guarantee should be recorded as `claimed` until T-2 is closed. --- --- ## Remediation status Recorded after the fixes were implemented. Each entry states what was actually changed, and what was **not**, so a re-audit can verify rather than take it on trust. Every closure below is exercised by the platform integration suite against a real Postgres — 144 checks, up from 100. | # | Status | What changed | | --- | --- | --- | | **T-1** | ✅ Closed | The event primary key is now `(arende_id, id)`. There is no shared namespace left to occupy, so the cross-tenant attack has no surface rather than a mitigation. Within a case, a collision is judged on a **client digest** — a SHA-256 of the canonicalised body the client sent, taken before the server's own fields and before encryption, because neither the row nor the payload can be compared (the server timestamp always differs on a resend, and the ciphertext is new every time). Identical content stays idempotent and answers `200`; the same id with different content answers `409` and writes **nothing**, not even the rest of the batch. The same attack one level up — case ids are equally predictable — is closed separately: a case id already owned by another organisation answers `409` instead of silently doing nothing, which previously left the victim's case uncreated and every later sync answering `404`. The **adversarial-tenant harness** the audit recommended now exists and runs in CI. | | **T-2** | ✅ Closed | `matdonId` is resolved against the `matdon` register for the caller's organisation, and the designation and calibration date are **derived from the register**, overwriting whatever the client sent. An unknown instrument is rejected with `400`. An expired one is not — the measurement was still taken, and refusing it would destroy evidence — but the register's date is what travels, so the grade falls to E1 on the register's word rather than the client's. A measurement with no instrument at all has any claimed designation or calibration **stripped**: it cannot be substantiated. | | **T-3** | ◐ Reduced, not closed | Subject keys are now stored **enveloped** under `PERSONNYCKEL_HUVUD`, a 32-byte master key held in Secrets Manager and injected via External Secrets — never in the database. A restored dump yields keys that do not open, verified by restarting the service without the master key and confirming the identifier cannot be read. **What remains, stated plainly:** a backup taken *before* an erasure, combined with the master key, still restores the data. Closing that needs a key per subject in a KMS where destruction is irreversible. The envelope is the preparation for it — wrap and unwrap are two functions, not inline code — but the erasure promise is still bounded by backup retention, and must be stated as such to data subjects. | | **T-4** | ✅ Closed | `services/plattform/gallring.mjs` destroys the keys of subjects whose cases have **all** passed `gallras_efter`, and records each destruction in `raderingar`. The grouping is on the blinded vehicle index, not the case id: a vehicle seen five times has five cases and one key, and gallringen on the oldest would have made the four newer ones unreadable early. A case with no retention date counts as not yet passed — early erasure cannot be undone. Scheduled as a Kubernetes CronJob, nightly, `concurrency_policy = Forbid`. | | **T-5** | ✅ Closed | Model-read instrument values carry `kalla`, declared on the domain type and rendered in the evidence report, so a machine-read number is distinguishable from one the technician read off the instrument. | | **T-6** | ✅ Closed | `gen_salt('bf', 12)`. The test that was supposed to lock this asserted the literal string `gen_salt('bf')` — it would have passed the weak variant forever. It now parses the cost out of every call site and requires ≥12. | | **T-7** | ✅ Closed | `atkomstlogg` and `raderingar` are append-only by the same trigger as the event log. `personnycklar` gets the narrower rule it needs: the key and the erasure-requested timestamp may change, identity and ownership may not — a key must not be movable to another subject. **The first version of this test was worthless**: a row-level trigger fires per row, so `delete` on an empty table succeeds without the protection ever being exercised. The test now requires the table to be non-empty before it proves anything. | | **T-8** | ✅ Closed | `/api/atkomstlogg` is documented. The spec's own description of idempotent sync was rewritten — it still claimed a known id is ignored silently, which is no longer true and was the behaviour T-1 depended on. | | **T-9** | ✅ Closed | An **external** rule package (`ECM_REGLER_FIL` set) without a signature now blocks case closure, exactly as an invalid signature does. The built-in default ships with the image and needs no signature — what needs one is a package somebody mounted in, because that is precisely the attack. | | **T-10** | ✅ Closed | An unset origin list now means same-origin, not `*`. Opening up requires writing `*` and meaning it. The decision lived at six call sites, any one of which could fall back to `*`; it is now one helper that either emits the headers or emits nothing. | | **T-11** | ✅ Closed | `exp` is required, not merely checked when present. | | **T-12** | ◐ Traded, with reasons | Upgraded to `react-router-dom` 7.18.2, which closes the applicable advisory (open redirect via backslash in `Link`/`useNavigate`). This introduces one **high** advisory that does not apply to this build: it concerns RSC mode, and the application uses `BrowserRouter`/`HashRouter` with no RSC imports, no server actions and no SSR — verified by search. No published version is clean of both; `npm audit fix --force` would downgrade to 7.11.0 and reinstate the applicable one. Real exposure is reduced; the audit line looks worse. Verified across the full suite, the case walkthrough and the portal-guard run. | ### What the hardening itself exposed Two of the fixes were briefly wrong in the same way the findings were, and both were caught only by trying to prove them: - The **T-7 test passed against an empty table.** A row-level trigger has no rows to fire on, so `delete` succeeded and the check reported green. It was measuring nothing. - The **T-6 test asserted the call, not the cost.** `expect(plattform).toContain("gen_salt('bf')")` would have accepted cost 6 indefinitely — and did, for as long as it existed. Both are the pattern the audit named: a control that is correct in its own terms and unexamined at its boundary. It applies to tests as readily as to code. ### Residual | Item | Why it is still open | | --- | --- | | **T-3** (partial) | A pre-erasure backup plus the master key still restores the data. Needs per-subject keys in a KMS with irreversible destruction. Until then, the backup retention window is part of the erasure promise and must be stated to data subjects. | | **T-12** (partial) | No `react-router` version is free of both advisories. The one carried does not apply to this build. | | Terraform | The CronJob and the secret wiring are written but **not applied** — no Terraform binary was available in this environment, so they are unvalidated beyond review. | | Rev 1 · C-4 | Processor agreement and transfer impact assessment. Documents to be written and signed, not code. | | Rev 1 · m-4 | Retiring the Supabase orchestrator. A deployment decision. | | Rev 1 · m-6 | Manual accessibility review. | ### Re-audit verdict Two of three Critical findings are closed at the structural level rather than mitigated: T-1 has no attack surface left, and T-2 derives from the register instead of trusting the caller. The third is materially reduced and honestly bounded. Applying the product's own standard: measurement traceability moves from `claimed` to `validated`. Erasure stays at `tested` — the envelope closes the "key beside the ciphertext" defect but not the backup window, and it should not be recorded as `validated` until it does. --- *ALVA-DOC-0003 · Internal engineering review · Not endorsed by any inspection body*