alva/felsokning/docs/TUV-AUDIT.md

# 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.

---

*ALVA-DOC-0003 · Internal engineering review · Not endorsed by any inspection body*