Files
alva/felsokning/docs/QUALITY-AUDIT.md
T
Claude 03fbea8fac Revisionen: m-4 och m-6 — de två som bara går att stänga delvis
m-6 · Tillgänglighetstester med axe på verkstadsgränssnittets
grundkomponenter: knapp, textfält, panel och bild. Gränssnittet var
genomtänkt för handskar, buller och solljus men obevisat — ingenting
kontrollerade att en knapp har ett tillgängligt namn eller att ett
formulärfält har en etikett.

Att bilder bär alternativtext är i den här produkten inte bara ett
tillgänglighetskrav: bilden *är* bevisningen, och beskrivningen är det
som säger vad den visar.

Ungefär en tredjedel av WCAG går att kontrollera maskinellt. Testet
säger det rakt ut, och den manuella granskningen står kvar som öppen —
automatik hittar inte om ett gränssnitt är användbart, bara om det är
felformat.

m-4 · Paritetstestet mellan orkesterkopiorna jämför nu innehåll i
stället för att kontrollera att strängar förekommer: grundreglerna ord
för ord, metodikkatalogens id i ordning, och modellvalet per uppgift.
Ett substrängtest hade inte fångat att en grundregel ändrats i ena
kopian.

Den riktiga åtgärden är att avveckla Supabase-vägen. Det är ett
driftbeslut, inte en kodändring, och står kvar som öppet.

Revisionsdokumentet har fått en avdelning med utfall per fynd, så att en
omgranskning kan verifiera i stället för att lita på ordet. Två punkter
är ärligt öppna och ingen av dem är kod: personuppgiftsbiträdesavtalet
med konsekvensbedömning, och avvecklingen av den äldre orkestern.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012EQg3rJsrQ1ZNTvkzmQAtt
2026-08-05 13:48:06 +00:00

22 KiB
Raw Blame History

Supplier quality audit — Guidad Felsökning

Framing. This review was performed at the request of the product owner, applying the lens an OEM quality and supplier-assurance function would use (Volkswagen AG named as the reference customer). It is an internal engineering review authored by the development team — not an audit conducted by, commissioned by, or endorsed by Volkswagen AG, and it carries no external standing.

Scope: commit 031d6d5, branch claude/guidad-felsokning-vision-1mnx7f. Basis: source code, not documentation claims. Where the two disagree, the code is reported.


Summary

The product is unusually well engineered for its stage. The append-only model, the allowlist sharing boundary, split IAM roles, immutable image tags and the refusal to state conclusions without evidence are all above what this reviewer normally sees in a workshop-tooling supplier.

However, the central product claim does not hold at the API boundary. The documentation states: "The system can never write a conclusion that ECM has not approved" and "the history is what gives the record its value in a dispute." Neither is true as implemented. The quality gate exists only in the browser, and the two fields that establish evidentiary provenance — who did the work and when — are supplied by the client and accepted unverified.

For a tool whose commercial premise is that its records hold up in warranty and insurance disputes, that is the finding that decides everything else.

Severity Count Meaning
Critical 4 Blocks supplier approval. Must be closed before any pilot with customer data.
Major 6 Blocks series deployment. Closable within one release cycle.
Minor 7 Track and schedule.

Critical findings

C-1 · Event attribution and timestamps are client-controlled

services/plattform/server.mjs:11761185

if (typeof post?.id !== "string" || !post.tidpunkt || typeof post.anvandare !== "string" || !post.handelse) {
  return svara(res, 400, { error: "Ogiltig händelse." });
}
await pool.query(
  `insert into felsokning_handelser (id, arende_id, tidpunkt, anvandare, handelse)
   values ($1, $2, $3, $4, $5) on conflict (id) do nothing`,
  [post.id, handelserVag[1], post.tidpunkt, post.anvandare, post.handelse],
);

The server checks that anvandare is a string and that tidpunkt is truthy. It does not check that either is true.

Consequences, all reachable with an ordinary technician token and curl:

  • Work can be attributed to a colleague who was not present.
  • Work can be backdated or forward-dated at will. The same field drives the billing time distribution shown to the customer.
  • The JWT already carries the authenticated identity (sub, namn). It is ignored on this path.

An append-only log whose author and time fields are attacker-controlled is not an audit trail. It is a signed container for unverified assertions. In a warranty dispute, opposing counsel needs one sentence to establish that.

Required: derive anvandare from the verified JWT and reject any client-supplied value. Set tidpunkt server-side; if the client's clock matters for offline work, accept it as a separate registrerad_tidpunkt field and store the server's receipt time alongside it, so the gap is visible rather than invisible.

C-2 · The ECM quality gate is a client-side control only

app/src/felsokning/ecm.ts:730 · app/src/pages/felsokning/ArendeSida.tsx:2214

grindGodkand() runs in the browser. The server accepts arende_avslutat — and every other event type — with no reference to the gate.

A case can therefore be closed with zero evidence, no root-cause analysis, no symptom verification and no customer decision, by posting one event to the API. The printed report and the traceability package will present that case as complete.

This directly contradicts the documented control. Every compliance guarantee built on top of ECM — warranty rule packs, insurance evidence requirements, the customer-approval interlock — inherits the same weakness.

Required: evaluate the gate server-side on arende_avslutat and reject the event when it fails. The client-side gate stays as user guidance; it must not be the enforcement point. This also means ECM has to run on the server, which argues for extracting it into services/gemensam/ alongside the observability module.

C-3 · No lawful basis for erasure, retention or data-subject requests

infra/postgres-init.sql:127133 · no deletion path in the API

The design is append-only in three layers, deliberately and with good reason. There is no corresponding mechanism for:

  • GDPR Art. 17 erasure. Customer names, telephone numbers, e-mail addresses, registration numbers and VINs are personal data. VIN in particular is treated as personal data by EDPB guidance when it can be linked to a person, which is precisely what this system does. There is no path to erase them, and the database trigger actively prevents it.
  • Retention limits (Art. 5(1)(e)). Nothing expires. inloggningsforsok is cleaned after a day; case data is kept forever by construction.
  • Data-subject access requests. No export scoped to one person.
  • Pseudonymisation (Art. 32). Technician names are stored in clear in every event.

"Append-only" and "erasure on request" are not irreconcilable, but they must be reconciled deliberately. The standard resolution is crypto-shredding: store personal identifiers encrypted with a per-subject key, and erase by destroying the key. The log stays intact and hash-verifiable; the personal data becomes unrecoverable. The evidentiary chain survives because what is erased is identification, not the record of what was checked.

Required: a documented retention and erasure design before any deployment with real customer data, plus a DPIA. This is not a feature request — without it the product cannot lawfully be sold to an EU workshop.

C-4 · Third-country transfer of customer and vehicle data is undocumented

services/ai-orkester/server.mjs · docs/OPERATIONS.md

The work-order scan sends a photograph of the work order — customer name, address, telephone, e-mail, registration number, VIN — to the Anthropic API. The guidance and review tasks send the case brief, which contains the same data plus the fault description.

The operations document states there are "no external service dependencies beyond the Anthropic API for the model calls." That is technically accurate and legally the whole issue: it is the one dependency that processes personal data outside the workshop's control.

Absent from the repository: a data processing agreement reference, a transfer impact assessment, the processing location, the retention behaviour of the processor, and whether the customer is informed. There is also no configuration to run the platform with the model calls disabled for customers who cannot accept the transfer — the methodology engine would function alone, so this is achievable.

Required: document the processor relationship and the transfer basis; add a per-organisation setting that disables model calls entirely; state in the customer-facing report when a model contributed to the record.


Major findings

M-1 · Measured values carry no instrument identity or calibration status

app/src/felsokning/ecm.ts:3139 · app/src/felsokning/domain.ts

The evidence hierarchy rates a measured value E4 — "high" — above a photograph. The matvarde event carries beskrivning, varde and enhet. It does not carry which instrument produced the value, its serial number, or whether that instrument was within calibration on the date of measurement.

In an OEM warranty audit, an uncalibrated measurement is not evidence of a lower grade; it is not evidence. A workshop claiming warranty on a measured out-of-tolerance value must be able to show the instrument was traceable to a standard. The system currently ranks such a value above a photograph that actually shows the fault.

Required: an instrument register per organisation (designation, serial, calibration due date) and an instrument reference on matvarde. Downgrade a measurement to E1 when no calibrated instrument is referenced, and say so in the report rather than silently claiming E4.

M-2 · High-voltage authorisation is self-attested, ungraded, and non-blocking

app/src/felsokning/metodiker.ts:577593

The safety step asks "Har du behörighet för arbete på högvoltsystem?" as a yes/no question. Three problems, in ascending order of seriousness:

  1. No competence level. OEM practice distinguishes graded qualifications (instructed person / qualified for de-energised work / qualified for work under voltage). "Authorised" is not a single thing, and the level determines which steps are permissible.
  2. No expiry and no register. The answer is a checkbox, not a lookup against a held qualification with a validity date.
  3. Answering "No" blocks nothing. nastaSteg() advances on any answer. A technician who honestly declares they are not qualified is walked into the next step of a procedure that can kill them.

The team's own stated rule is that where work can injure someone, the safety step comes first and cannot be skipped. It comes first. It can be skipped.

Required: make the safety step a hard interlock — a "No" must stop the methodology and offer escalation, not advance. Record the competence level and its validity on the organisation's user record, not as a question.

M-3 · No server-side event schema validation

services/plattform/server.mjs:1176

post.handelse is accepted as arbitrary JSON. An unknown or malformed typ enters the append-only log permanently and cannot be corrected, because the log is append-only. A client bug at one workshop permanently contaminates that organisation's evidentiary record.

The sharing allowlist fails safe here (an unknown type is not in DELBART_KUND, so it is filtered out), which is good design and limits the blast radius to internal views.

Required: validate handelse against the domain schema at the API boundary. The OpenAPI specification already defines these types; validate against it.

M-4 · No read audit trail

services/plattform/server.mjs

Every write is logged. No read is. There is no record of which technician, supervisor or administrator viewed which case, nor of who followed a share link.

For personal data this is an Art. 32 gap. For the product's own value proposition it is a missed opportunity: "the customer opened the share link three times before approving" is exactly the kind of fact a workshop wants when a customer later disputes having approved the work.

M-5 · Backup is monitored but never restore-tested; no RTO/RPO

infra/aws/50-doman-observation.tf:131147 · docs/OPERATIONS.md

The backup-age alarm is well designed — alarming on missing data rather than a threshold is a mature choice and the team documented why. But the alarm proves that backup occurs, not that it restores. There is no scheduled restore test, and no stated recovery time or recovery point objective.

The operations document says correctly that a lost event log destroys every case's probative value and cannot be recreated. That makes an untested restore the largest single-point risk in the system.

M-6 · Compliance rules are swappable without approval or integrity control

services/plattform/server.mjs:3441

The ECM rule pack is read from a file replaceable by a ConfigMap. This is a genuinely good design for operability — new warranty terms without a release.

But the rules govern what the system will accept as a compliant case. Anyone with namespace write access can change what "compliant" means, with no signature, no approval record, and no four-eyes control. The pack version is recorded in the traceability package, which is necessary but not sufficient: a version number nobody approved proves only that something was loaded.

Required: sign the rule pack and verify the signature on load; record who approved a pack version and when.


Minor findings

# Finding Location
m-1 Share code generation has modulo bias — randomBytes 0255 reduced % 36 makes the first four characters ~14 % more likely. ~82 bits remain, so not exploitable, but it signals the crypto path was not reviewed against a checklist. Use rejection sampling. server.mjs:299
m-2 Event id is client-supplied with on conflict do nothing. A client can pre-claim an id and silently suppress a later legitimate event. server.mjs:1183
m-3 EXIF stripping (including GPS) happens as a side effect of canvas re-encoding. It is correct, undocumented and untested — a future "preserve original quality" change would silently reintroduce location data into shared photographs. Lock it with a test. format.ts:2125
m-4 Two orchestrator copies kept in sync by test rather than shared code. Works today; a structural liability. ai-orkester/, supabase/functions/
m-5 No SBOM and no dependency scanning in CI, with 53 direct client dependencies. Required under most OEM supplier security baselines. .gitea/workflows/
m-6 No end-to-end tests and no accessibility testing. The UI is designed for gloves, noise and sunlight — commendable — but EN 301 549 conformance is unevidenced. app/
m-7 The model that answered is logged; the prompt version and rule-pack version in force at that moment are not both bound to the response. Two identical cases can receive different guidance with no way to reconstruct why. ai-orkester/server.mjs

Confirmed strengths

Stated because an audit that lists only faults gives a false picture, and because these should be protected in any remediation.

  • No write path to the vehicle. Diagnosis is photograph- and observation-based. This keeps the product outside UN R156 scope and removes an entire category of safety argumentation. It appears to be a deliberate choice and it is the right one.
  • Append-only enforced at the database. Triggers reject changes even for a misconfigured role. The application layer is not the only barrier.
  • Allowlist sharing. A forgotten event type becomes internal, not leaked. The test that requires every domain type to be classified is exactly the right control.
  • Content-addressed attachments with hash verification on read. Serving 409 on mismatch rather than showing the image is the correct failure mode.
  • Split build and deploy roles; immutable registry tags; IRSA with per-account roles; IMDSv2 hop limit 1. This is a stronger cloud posture than most suppliers present at series maturity.
  • generisk as an honest fallback. A system that says "we do not know where to start" instead of guessing is a system whose confident statements can be trusted. This is a quality property, not a limitation.
  • The alarm on missing backup data. Reasoning about the failure mode rather than the metric.

The order matters more than the list. C-1 and C-2 are cheap to fix and everything else depends on them being true.

Before any pilot with real data

  1. C-1 — derive anvandare and tidpunkt server-side. Roughly a day, including a migration note that historical rows cannot be re-attributed.
  2. C-2 — move ECM to services/gemensam/ and enforce the gate on arende_avslutat. Reuses existing pure functions.
  3. M-3 — validate events against the OpenAPI schema at the boundary. Same commit as C-1.
  4. M-2 — make the high-voltage safety step a hard interlock. Small change, and the one whose absence could kill someone.

Before series deployment

  1. C-3 — retention and erasure design (crypto-shredding), plus a DPIA.
  2. C-4 — processor documentation and a per-organisation model-off switch.
  3. M-1 — instrument register and calibration-aware evidence grading.
  4. M-5 — scheduled restore test with a stated RTO/RPO.
  5. M-6 — signed rule packs with an approval record.
  6. M-4 — read audit trail.

Continuous

  1. The minor findings, m-1 and m-3 first — both are single-commit changes and m-3 protects a property that currently holds only by accident.

Closing assessment

The engineering judgement in this codebase is consistently good, and in several places better than the standard the product is being measured against here. The reasoning recorded in the commit history and in the design-decision table is of a quality that makes an audit unusually easy to conduct — motives are written down, so it is possible to check whether the implementation matches the intent rather than merely whether it works.

That is what makes C-1 and C-2 worth stating plainly rather than diplomatically. They are not oversights of ambition; they are the gap between a system that documents evidence and a system that guarantees it. The product is sold on the second claim. Closing that gap is roughly two days of work, and until it is closed every other guarantee in the documentation rests on a client that anyone can bypass with curl.

Close C-1 and C-2, and the honest description of this product changes from "a well-built diagnostic app" to "an evidentiary system". That is a different market and a different price.


Remediation status

Recorded after the fixes were implemented. Each entry states what was actually changed, so a re-audit can verify rather than take it on trust.

# Status What changed
C-1 Closed anvandare derives from the verified JWT and tidpunkt from the server clock (services/gemensam/handelser.mjs). The client's timestamp is preserved as registrerad_tidpunkt so offline work is not lost and the gap is visible.
C-2 Closed The gate moved to services/gemensam/grind.mjs and runs on the server for arende_avslutat; a failing close returns 409 with the actual blockers. The methodology library moved to services/gemensam/metodiker.mjs; the client re-exports it typed, so there is still one source.
C-3 Closed Crypto-shredding (services/gemensam/personuppgifter.mjs): identifying fields encrypted per case key, erasure by destroying the key. A blinded HMAC index on the case row makes erasure reach a vehicle's whole history without storing the identifier in clear. Retention dates set at close per case type. POST /api/radering requires an exact confirmation and logs the erasure without recording the subject.
C-4 Closed ai_tillaten per organisation, carried in the JWT; the orchestrator refuses with 403 when false. The methodology engine works alone, so a workshop that cannot accept the transfer can still use the product. Still required and not a code change: the processor agreement and transfer impact assessment.
M-1 Closed matvarde carries matdonId, designation and calibration date; an instrument register lives at /api/matdon. Without a traceable, in-calibration instrument the value is graded E1 rather than E4, and the report says why. Calibration is judged at the time of measurement, not today.
M-2 Closed SPARRFRAGOR makes the high-voltage questions hard interlocks: a "No" stops the methodology with an explicit instruction to hand over, and the server refuses to close. An unanswered safety question also blocks — silence is not a yes.
M-3 Closed Every event is validated against HÄNDELSESCHEMA before writing. Unknown types are rejected. A test additionally requires every schema type to be classified in the sharing allowlist.
M-4 Closed atkomstlogg records reads with user, case, route, source and share code; readable at /api/atkomstlogg for supervisors and admins.
M-5 Closed services/plattform/aterstallningstest.sh runs in CI: dump, restore into an empty database, then verify events, provenance, ordering, attachment hashes, person keys — and that the append-only triggers both survived and still bite.
M-6 Closed The rule pack is verified against an HMAC. An invalid signature blocks closing; a missing key gives audit mode with a warning rather than an outage, because that is how security features get switched off.
m-1 Closed Rejection sampling replaces modulo in nyKod.
m-2 Closed Id collisions are counted and surfaced in the trace rather than passing silently.
m-3 Closed EXIF/GPS stripping is now documented as deliberate and locked by a test that reads the code path, not just the output.
m-4 ◐ Partial The parity test now compares content — base rules word for word, catalogue ids in order, model routing per task — instead of checking that strings appear. The real fix is retiring the Supabase path, which is a deployment decision, not a code change.
m-5 Closed npm audit at high severity plus a CycloneDX SBOM per build, archived 400 days.
m-6 ◐ Partial Automated accessibility tests with axe on the core workshop components. Roughly a third of WCAG is machine-checkable; the manual review remains outstanding.
m-7 Closed Each response carries a promptversion — a short stable hash of model, effort and system prompt — so two answers can be compared without storing the prompt.

What remains

Two items are honestly open, and neither is a code change:

  1. The processor agreement and transfer impact assessment (C-4). The technical control exists; the legal basis is a document that has to be written and signed, and a DPIA that has to be performed.
  2. Retiring the Supabase orchestrator (m-4). Duplication defended by tests is a liability that grows. The path is documented as the older one; the decision to remove it belongs to the product owner.

A manual accessibility review (m-6) should also be scheduled. Automated tooling does not find whether the interface is usable, only whether it is malformed.

Re-audit verdict

The two findings that decided the assessment — C-1 and C-2 — are closed at the API boundary, which is where they had to be closed. The record now establishes who did the work and when from the authenticated session rather than from the caller, and a case cannot be closed without the evidence the product claims to require.

The honest description of this product has changed from "a well-built diagnostic app" to "an evidentiary system." Subject to the two open documents above, it would pass supplier assessment for a pilot with customer data.