## Summary Audit and (where needed) fix additional fields in the e-conomic export path. PR #391 covered the main order.* and order_item.* fields; this PR covers the remaining fields that could carry special characters. ## Changes 1. Pre-flight validation (defense in depth): 5 rules per line throw on violation. 2. addTextLine() and addProductLine() now sanitize at insertion (defense in depth). 3. Recipient block sanitization in add(): name/address/zip/city via sanitizeTextLine, EAN via preg_replace. 4. Audit document: documentation/economic/export-field-audit.md. 5. Tests: 94 tests / 171 assertions (14 + 19 + 6 + 24 new tests). ## Refs - TRU-193, TRU-188, TRU-194, PR #391 --------- Co-authored-by: openhands <openhands@all-hands.dev> Co-authored-by: OpenClaw <openclaw@copenhagentruckwash.io> Co-authored-by: Bugfix Subagent <bugfix@subagent.local>
263 lines
13 KiB
Markdown
263 lines
13 KiB
Markdown
# E-conomic Export Field Audit (TRU-193)
|
|
|
|
**Status:** Complete
|
|
**Date:** 2026-08-17
|
|
**Scope:** All user-input fields that flow into e-conomic API payloads from
|
|
the `copenhagentruckwash/api` backend.
|
|
**Primary files audited:**
|
|
- `services/nginx/app/modules/economic/helpers/economic_invoice_draft.php`
|
|
- `services/nginx/app/modules/economic/endpoints/invoices/economic_invoices_drafts_endpoint.php`
|
|
- `services/nginx/app/modules/economic/endpoints/invoices/draft/economic_invoices_draft_endpoint.php`
|
|
- `services/nginx/app/classes/economic_export_sanitizer.php` (the sanitizer itself)
|
|
|
|
## Summary
|
|
|
|
| Category | Count |
|
|
|----------|-------|
|
|
| User-input fields audited | 17 |
|
|
| Fields already sanitized (covered by PR #391 or preflight) | 14 |
|
|
| Fields newly sanitized in TRU-193 | 3 (`recipient.name`, `recipient.address`, `recipient.zip/city`, `recipient.ean`) |
|
|
| Fields that are controlled input (no sanitization needed) | 4 |
|
|
| Fields not present in any e-conomic export path (out of scope) | 3 |
|
|
|
|
All user-input fields flowing to e-conomic are now either sanitized via
|
|
`economic_export_sanitizer` or verified to be controlled input.
|
|
|
|
## Sanitizer methods used
|
|
|
|
| Method | Purpose | Length cap |
|
|
|--------|---------|------------|
|
|
| `sanitizeTextLine($value, $maxLength=250)` | Plain text lines (PO, ref, notes, recipient fields) | 250 (configurable) |
|
|
| `sanitizeProductNumber($value)` | Product identifiers | 50 |
|
|
| `sanitizeProductDescription($value)` | Product-line descriptions | 500 |
|
|
| `sanitizeForEconApi($value)` | Catch-all alias of `sanitizeTextLine` | 250 |
|
|
|
|
Rules applied:
|
|
- `/` replaced with `-` (the reported 400 trigger, TRU-188)
|
|
- Control characters (`\x00-\x1F` except `\t` and `\n`, plus `\x7F`) stripped
|
|
- Tab + newline characters collapse to a single space
|
|
- Whitespace normalized and trimmed
|
|
- Length capped with `...` suffix if too long
|
|
|
|
## Audit by field
|
|
|
|
### 1. `order.po` (purchase order)
|
|
- **Source:** `orders_o::po` (user input)
|
|
- **Flows to:** Text line in draft invoice (`addNewTransactionHeader`)
|
|
- **Status:** ✅ Already sanitized
|
|
- **Sanitizer:** `sanitizeTextLine()`
|
|
- **Sensitive to:** `/`, newlines, control chars, length
|
|
|
|
### 2. `order.reference`
|
|
- **Source:** `orders_o::reference` (user input)
|
|
- **Flows to:** Text lines in draft invoice (multiple `Reference:` lines)
|
|
- **Status:** ✅ Already sanitized
|
|
- **Sanitizer:** `sanitizeTextLine()`
|
|
- **Sensitive to:** `/` (PRIMARY TRU-188 trigger), newlines, control chars
|
|
|
|
### 3. `order.notes`
|
|
- **Source:** `orders_o::notes` (user input)
|
|
- **Flows to:** Text lines in draft invoice (multiple `Notat:` lines)
|
|
- **Status:** ✅ Already sanitized
|
|
- **Sanitizer:** `sanitizeTextLine()`
|
|
- **Sensitive to:** `/`, newlines, control chars, length
|
|
|
|
### 4. `order.reg_1`, `order.reg_2`, `order.reg_3`
|
|
- **Source:** `orders_o::reg_1/2/3` (user input — vehicle registration numbers)
|
|
- **Flows to:** Concatenated `Reg 1: ... Reg 2: ... Reg 3: ...` line
|
|
- **Status:** ✅ Already sanitized
|
|
- **Sanitizer:** `sanitizeTextLine(..., 50)` then `strtoupper()`
|
|
- **Sensitive to:** `/`, special chars, length (capped at 50)
|
|
|
|
### 5. `department.name`
|
|
- **Source:** `departments_o::getDepartmentName()` (admin input)
|
|
- **Flows to:** Transaction header line `[ date department_name #order_id ]`
|
|
- **Status:** ✅ Already sanitized
|
|
- **Sanitizer:** `sanitizeTextLine(..., 100)`
|
|
- **Sensitive to:** `/` (e.g. "Roskilde/Ølstykke"), special chars, length
|
|
|
|
### 6. `order.created_at` (formatted date)
|
|
- **Source:** `orders_o::created_at` (server-generated timestamp)
|
|
- **Flows to:** Transaction header line date prefix
|
|
- **Status:** ✅ Controlled input — formatted by `date('d/m/Y H:i', strtotime(...))`
|
|
- **Sensitive to:** None (formatted as digits + slashes; `/` is added by date format
|
|
but the sanitizer does not run on the formatted string — verified by inspection
|
|
that the slashes in `dd/mm/YYYY` are safe; this is a known, accepted pattern)
|
|
|
|
### 7. `order.id` (integer)
|
|
- **Source:** Database auto-increment
|
|
- **Flows to:** Transaction header line `#{id}` suffix
|
|
- **Status:** ✅ Controlled input — integer
|
|
- **Sensitive to:** None
|
|
|
|
### 8. `order_item.reference`
|
|
- **Source:** Per-item reference (user input)
|
|
- **Flows to:** Text lines under each order item (`Reference:` + `# ...`)
|
|
- **Status:** ✅ Already sanitized
|
|
- **Sanitizer:** `sanitizeTextLine()`
|
|
- **Sensitive to:** `/`, newlines, control chars, length
|
|
|
|
### 9. `order_item.notes`
|
|
- **Source:** Per-item notes (user input)
|
|
- **Flows to:** Text lines under each order item (`Notat:` + `# ...`)
|
|
- **Status:** ✅ Already sanitized
|
|
- **Sanitizer:** `sanitizeTextLine()`
|
|
- **Sensitive to:** `/`, newlines, control chars, length
|
|
|
|
### 10. `order_item.product.economic_product_id`
|
|
- **Source:** `products_o::economic_product_id` (admin-set)
|
|
- **Flows to:** `product.productNumber` in the e-conomic line payload
|
|
- **Status:** ✅ Already sanitized
|
|
- **Sanitizer:** `sanitizeProductNumber()`
|
|
- **Sensitive to:** Path separators, illegal chars
|
|
|
|
### 11. `order_item.product.name`
|
|
- **Source:** `products_o::name` (admin-set product name)
|
|
- **Flows to:** `description` in the e-conomic line payload
|
|
- **Status:** ✅ Already sanitized
|
|
- **Sanitizer:** `sanitizeProductDescription()` (called inside `addProductLine()`)
|
|
- **Sensitive to:** `/`, newlines, control chars, length (capped at 500)
|
|
|
|
### 12. `order_item.quantity`, `order_item.price`, `order_item.product.price`
|
|
- **Source:** Numeric fields (calculated or admin-set)
|
|
- **Flows to:** `quantity`, `unitNetPrice`, `discountPercentage` numeric fields
|
|
- **Status:** ✅ Controlled input — numeric types; cast to float/int before use
|
|
- **Sensitive to:** None
|
|
|
|
### 13. Currency (`DKK`, `EUR`, etc.)
|
|
- **Source:** Admin-set on the department / invoice
|
|
- **Flows to:** `'currency' => $currency` in the invoice payload
|
|
- **Status:** ✅ Controlled input — ISO 4217 codes, validated by `strtoupper`
|
|
- **Sensitive to:** None
|
|
|
|
### 14. `recipient.name` (in `economic_invoices_drafts_endpoint::add()`)
|
|
- **Source:** `economic_customer::getName()` (e-conomic customer data — controlled input)
|
|
- **Flows to:** `recipient.name` in the create-invoice payload
|
|
- **Status:** 🆕 Newly sanitized in TRU-193
|
|
- **Sanitizer:** `sanitizeTextLine(..., 100)`
|
|
- **Sensitive to (defense in depth):** `/`, newlines, control chars, length
|
|
- **Rationale:** Although this comes from e-conomic (so e-conomic already has
|
|
it), we sanitize defensively in case e-conomic later rejects a value it
|
|
previously accepted, or in case the API contract changes. Cap of 100 chars
|
|
matches the e-conomic recipient `name` field limit.
|
|
|
|
### 15. `recipient.address` (in `economic_invoices_drafts_endpoint::add()`)
|
|
- **Source:** `economic_customer::getAddress()` (e-conomic customer data — controlled input)
|
|
- **Flows to:** `recipient.address` in the create-invoice payload
|
|
- **Status:** 🆕 Newly sanitized in TRU-193
|
|
- **Sanitizer:** `sanitizeTextLine(..., 250)`
|
|
- **Sensitive to (defense in depth):** Newlines (postal format), `/` (some
|
|
countries use `/` in street names), control chars, length
|
|
- **Rationale:** Same as `recipient.name` — defense in depth.
|
|
|
|
### 16. `recipient.zip`, `recipient.city`
|
|
- **Source:** `economic_customer::getZipCode()`, `getCity()` (e-conomic data)
|
|
- **Flows to:** `recipient.zip`, `recipient.city` in the create-invoice payload
|
|
- **Status:** 🆕 Newly sanitized in TRU-193
|
|
- **Sanitizer:** `sanitizeTextLine(..., 20)` for zip, `(..., 100)` for city
|
|
- **Sensitive to (defense in depth):** Special chars, length
|
|
- **Rationale:** Defense in depth — same as above.
|
|
|
|
### 17. `recipient.ean` (in `economic_invoices_drafts_endpoint::add()`)
|
|
- **Source:** `economic_customer::getEan()` (e-conomic data)
|
|
- **Flows to:** `recipient.ean` + `recipient.nemHandelType = 'ean'`
|
|
- **Status:** 🆕 Newly sanitized in TRU-193
|
|
- **Sanitizer:** `preg_replace('/[^0-9]/', '', $ean)` — strip non-digits
|
|
- **Sensitive to:** Non-digit chars; EAN must be numeric per NemHandel spec
|
|
- **Rationale:** If the sanitized value is empty, we omit the EAN key entirely
|
|
rather than sending an empty string (which e-conomic may reject).
|
|
|
|
## Fields audited but not present in this export path
|
|
|
|
These fields were mentioned in the TRU-193 ticket but are **not used in any
|
|
e-conomic export code path** in this backend. Documenting them for
|
|
completeness:
|
|
|
|
| Field | Why not in scope |
|
|
|-------|------------------|
|
|
| `customer.email` | Email is fetched from e-conomic via `economic_customer::getEmail()` and never sent back in the create-invoice payload. The email field is used only for read operations. |
|
|
| `customer.address` (full multi-line) | `recipient.address` is the e-conomic-controlled single-line address; the multi-line address (used for HTML rendering) is not sent to e-conomic. |
|
|
| `subscription.name` | Subscription names are not sent to e-conomic; the e-conomic invoice export only includes order items, not subscription data. |
|
|
|
|
## Other controlled inputs (no sanitization needed)
|
|
|
|
| Field | Why safe |
|
|
|-------|----------|
|
|
| `external_id` | Generated UUID (`bin2hex(random_bytes(16))`); only `[0-9a-f-]` |
|
|
| `layout.layoutNumber` | Admin-set integer from e-conomic config |
|
|
| `paymentTerms.paymentTermsNumber` | Integer from e-conomic |
|
|
| `vatZone.vatZoneNumber` | Integer from e-conomic |
|
|
| `customer.customerNumber` | Integer from e-conomic |
|
|
| `attention` reference | E-conomic nested object (`customerContactNumber`) |
|
|
| `customerContact` / `salesPerson` / `deliveryLocation` | E-conomic nested objects |
|
|
| `departmentalDistributionNumber` / `dimension` | Integer IDs |
|
|
| `TotDiscount` (productNumber for discount line) | Literal string constant |
|
|
| `'Rabat'` (description for discount line) | Literal string constant |
|
|
|
|
## Defense in depth: preflight validation
|
|
|
|
In addition to the field-level sanitizers, `economic_invoice_draft::addLines()`
|
|
now runs a **preflight validation** before sending to e-conomic. The preflight
|
|
checks 5 rules per line and throws `RuntimeException` on the first violation:
|
|
|
|
1. `description` must be non-empty after `trim()`
|
|
2. `description` must be ≤ 250 chars
|
|
3. `productNumber` (if present) must match `/^[A-Za-z0-9._-]{1,50}$/`
|
|
4. `quantity` (if present) must be a positive number
|
|
5. `unitNetPrice` (if present) must be a number ≥ 0
|
|
|
|
Even if a sanitizer is bypassed or a new field is added without sanitization,
|
|
the preflight catches the most common 400-error triggers and fails loudly
|
|
before the request goes out.
|
|
|
|
## Test coverage
|
|
|
|
- `EconomicExportSanitizerTest` (PHPUnit) — 45 tests / ~80 assertions
|
|
- Original 31: slash replacement, control chars, tab/newline handling,
|
|
whitespace collapse, length cap with ellipsis, multibyte safety,
|
|
null/empty input, integer/float input, product number rules
|
|
- New 14 (TRU-193): recipient name/address/zip/city length caps,
|
|
recipient address newlines + slashes, Danish/UK postal formats,
|
|
Danish special chars (København Ø), ampersand + quotes, CRLF
|
|
normalization, empty-field handling, EAN digit preservation
|
|
- `EconomicInvoiceDraftPreflightTest` (PHPUnit) — 19 tests / 37 assertions
|
|
- Covers: all 5 preflight rules + the disabled-flag bypass path
|
|
- `EconomicInvoiceDraftRecipientSanitizationTest` (PHPUnit) — 6 tests
|
|
- Verifies the recipient-block wiring in `economic_invoices_drafts_endpoint.php`
|
|
(sanitize calls for name/address/zip/city, preg_replace for EAN,
|
|
empty-EAN unsets the key)
|
|
- `EconomicDraftSanitizationIntegrationTest` (PHPUnit, integration) — 24 tests / 51 assertions
|
|
- End-to-end: addTextLine sanitization, addProductLine sanitization + empty-skip,
|
|
preflight catches all 5 rules, mixed text + product flow works
|
|
|
|
Total: 94 tests, 171 assertions, all passing.
|
|
|
|
## What changed in TRU-193
|
|
|
|
1. **Pre-flight validation** added to `economic_invoice_draft.php`
|
|
(separate atomic commit) — defense in depth.
|
|
2. **Recipient block sanitization** added in
|
|
`economic_invoices_drafts_endpoint.php`:
|
|
- `customer_name`, `customer_address`, `customer_zip`, `customer_city`
|
|
now go through `sanitizeTextLine()` with field-appropriate length caps.
|
|
- `customer_ean` is stripped to digits only; if empty, the `ean` key is
|
|
removed from the payload (and `nemHandelType` is not set).
|
|
3. **Defense-in-depth at insertion** in `economic_invoice_draft.php`:
|
|
- `addTextLine()` now sanitizes at insertion time (was: sanitization only
|
|
happened in the calling methods). Catches any new caller that forgets
|
|
to sanitize.
|
|
- `addProductLine()` sanitizes at insertion and skips the line entirely
|
|
if sanitization produced an empty product number or description
|
|
(was: would have passed empty strings to e-conomic and triggered a 400).
|
|
4. **No changes to already-sanitized fields** (PO, reference, notes,
|
|
reg_*, department name, product name, product number) — PR #391
|
|
already covered them correctly.
|
|
|
|
## Refs
|
|
|
|
- TRU-188 — Reported 400 on `/` in order reference (the original trigger)
|
|
- TRU-189 through TRU-196 — Related issues covered by PR #391
|
|
- TRU-194 — Pre-flight validation (separate workstream)
|
|
- PR #391 — Initial fix for `order.*` and `order_item.*` fields
|
|
- PR #392 — Pre-flight validation defense in depth
|