# ZF (Zero-Friction) — Read-only audit

**Branch:** `retail-audit` (no code changes)
**Scope:** the ZF backend pipeline + FE pages — OCR, shelf scan, ZATCA submission, reconciliation, learned-SKU memory, settings.
**Companion docs:** `RETAIL_AUDIT.md`, `MERCHANDISING_AUDIT.md`.

ZF is positioned as a "set it and forget it" back-office layer:
auditor uploads a supplier invoice → OCR parses it → matched lines auto-create a
draft receiving → the auditor approves → inventory + journal posts.
Reconciliation, shelf scan, and ZATCA submission are sister kinds in the same jobs table.

This audit found that the framing is true for OCR. **It is not true for the other three kinds.**

---

## 1. Surface map

### Backend

| Component | File | Lines |
|---|---|---|
| Controller | `backend/app/Http/Controllers/Zf/ZfController.php` | 686 |
| OCR service (only non-stub) | `backend/app/Services/Zf/ZfOcrService.php` | 409 |
| Models | `ZfJob`, `ZfLearned`, `ZfReconciliation`, `ZfSettings`, `ZfShelfScan` | 5 files |
| Routes | `backend/routes/api.php:1153-1166` | 14 endpoints |

### Frontend

`app/src/modules/retail/zf/` — 11 pages, ~1,617 LOC total:

| Page | LOC | Purpose |
|---|---|---|
| `OcrPage.jsx` | 392 | Real — kicks off `kind=ocr` jobs, drives review modal |
| `ReconciliationPage.jsx` | 239 | Real — reads `/zf/reconciliation`, calls post-close |
| `SettingsPage.jsx` | 296 | Real — OCR provider config + feature flags |
| `JobsPage.jsx` | 104 | Real — paginated table from `/zf/jobs` |
| `OverviewPage.jsx` | 66 | Real — KPIs from `/zf/overview` |
| `LearnedPage.jsx` | 75 | Real — list from `/zf/learned` |
| `ShelfPage.jsx` | 64 | Reads real `/zf/shelf`; "Run shelf scan" creates a stub job |
| `ZatcaPage.jsx` | 84 | Reads real `ZatcaSubmission`; "Submit" creates a stub job |
| `ZfFlowPage.jsx` | 155 | Demo storyboard — not wired |
| `ZfShell.jsx` | 54 | Layout |
| `_OnboardingBanner.jsx` | 88 | Onboarding nudge |

---

## 2. Route security posture

ZF routes (`routes/api.php:1153-1166`) sit inside the generic
`['auth:sanctum', 'tenant', 'tenant_active']` group that starts at line 409.

What that gives us:

- ✅ Auth required
- ✅ Tenant context required
- ✅ Tenant must be active

What it does **not** give us:

- ❌ No `module:zf` entitlement gate (cf. `module:ecom`, `module:pay`, `module:dine`, `module:accounting`)
- ❌ Zero `authorizeP()` calls anywhere in `ZfController` — confirmed by grep
- ❌ No `requireStepUp` middleware on financial-posting endpoints
  (`/zf/reconciliation/{id}/post`, `/zf/jobs/{id}/ocr-approve`)
- ❌ No role hint in the route — a tenant cashier can call any ZF endpoint

`BelongsToTenant` global scope (`Concerns/BelongsToTenant.php:36-49`) does prevent
cross-tenant reads, so this is not a data-leak risk. It is, however, a
"who-can-touch-the-books" risk: any logged-in user inside a tenant can post
reconciliation journals, approve OCR drafts, and provision OCR vendor API keys.

---

## 3. Top findings (ranked)

### ZF-1 · HIGH · Three of four job kinds are silent stubs

`ZfController::jobsRun` at lines 115-130:

```php
$result = match ($zfJob->kind) {
    'ocr'          => app(ZfOcrService::class)->process($zfJob, $companyId), // real
    'reconcile'    => ['stubbed' => true, 'kind' => 'reconcile'],            // stub
    'shelf_scan'   => ['stubbed' => true, 'kind' => 'shelf_scan'],           // stub
    'zatca_submit' => ['stubbed' => true, 'kind' => 'zatca_submit'],         // stub
    default        => ['stubbed' => true, 'kind' => $zfJob->kind],
};
```

After the match, the job is unconditionally marked `status='done'` (line 133).
The FE Jobs page therefore renders a green "Done" badge for ZATCA submissions
that never reached ZATCA, shelf scans that never ran computer vision, and
reconciliations that didn't reconcile anything.

The `stubbed: true` flag is in the JSON result, but no FE surface checks for it.
A tenant who watches "Submit to ZATCA → Done" assumes their e-invoice cleared.
It did not.

**Why this matters:** ZATCA is a Saudi regulatory feed. Submitting an invoice
is a legal obligation. Showing a successful submission UI for a no-op is the
worst kind of bug — the operator believes they are compliant.

Files: `backend/app/Http/Controllers/Zf/ZfController.php:115-130`

---

### ZF-2 · HIGH · `jobsRun` swallows exceptions and returns HTTP 200

```php
} catch (\Throwable $e) {
    $zfJob->status      = 'failed';
    $zfJob->error       = substr($e->getMessage(), 0, 500);
    $zfJob->finished_at = now();
    $zfJob->save();
}
return ApiResponse::ok($this->shapeJob($zfJob)); // ← 200 regardless
```

The HTTP status is 200 even when execution failed. The FE must inspect
`status === 'failed'` to detect the failure.

Consequences:

- Generic HTTP-level retry logic (interceptors, `useMutation` `onError`)
  doesn't trigger on real failures.
- `console.error` / Sentry breadcrumbs that key off non-2xx responses
  miss every ZF failure.
- The OCR pipeline can blow up with a stack trace truncated to 500 chars
  and the API returns the same shape as a success.

This is the only place in the retail API surface (per audit) where the
controller actively converts a server-side exception into a 200.

Files: `backend/app/Http/Controllers/Zf/ZfController.php:136-143`

---

### ZF-3 · HIGH · No `module:zf` entitlement gate

Compare the ZF group (line 1153) with ecom (line 1134):

```php
Route::middleware('module:ecom')->group(function () { /* … */ }); // ecom is gated
// ZF routes — no group wrapper
Route::get ('/zf/overview', [ZfController::class, 'overview']);
```

Every other premium-feature module routes its endpoints through a `module:` gate.
ZF — which provisions third-party API keys, posts GL journals, and forwards
documents to a tax authority — does not.

Effect: A tenant on a free / overdue / non-ZF plan still has full access to ZF.
Plan changes can't suspend ZF without changing code.

Files: `backend/routes/api.php:1153-1166`

---

### ZF-4 · HIGH · `reconciliationPost` accepts attacker-controlled `actual`

`reconciliationPost` at lines 407-419:

```php
$data = $request->validate([
    'actual' => ['nullable', 'numeric', 'min:0'],
    'note'   => ['nullable', 'string', 'max:500'],
]);
// …
if (array_key_exists('actual', $data) && $data['actual'] !== null) {
    $reconciliation->actual   = (float) $data['actual'];
    $reconciliation->variance = round($reconciliation->actual - $reconciliation->expected, 2);
}
```

The controller comment claims this is "allow caller to supply the actual POS
amount at close time **if it wasn't stored yet**" — but the code never checks
whether the row already has a stored `actual`. It always overwrites.

Then immediately:

```php
$this->postZfJournal($reconciliation, $companyId, $variance);
```

…which posts a balanced journal whose DR/CR amount equals the variance the
caller just controlled (see lines 490-500). The journal sources are
`zf_reconciliation` with `source_ref = $r->id`, so the idempotency key won't
catch it (each new close is a new id… though re-closes are blocked by the
`status` check at line 398).

**Impact:** any user who can hit `/zf/reconciliation/{id}/post` can fabricate
COGS / Revenue debits and credits of arbitrary size. No step-up. No
permission check. The synthesised "actual" from POS data is overwritten by
whatever the request body says.

Files: `backend/app/Http/Controllers/Zf/ZfController.php:394-446`, `:455-510`

---

### ZF-5 · HIGH · OCR approval and recon-close miss step-up

`/zf/jobs/{id}/ocr-approve` posts a receiving (inventory write +
optional GL post via `ZfOcrService::approveReceiving`).
`/zf/reconciliation/{id}/post` posts a balanced journal.

Both routes are inside the standard group at line 1153. Neither has
`stepup:` middleware.

Compare with returns (`routes/api.php:697` per main audit item #4) which
already require step-up. ZF posting is structurally identical — the only
difference is the source key on the journal.

Files: `backend/routes/api.php:1157`, `:1164`

---

### ZF-6 · MEDIUM · No `authorizeP()` calls in `ZfController`

```bash
grep "authorizeP" backend/app/Http/Controllers/Zf/ZfController.php  # 0 results
```

Every other tenant controller scoping write-paths uses `$this->authorizeP('catalog.products.update')`
or equivalent. ZF uses none. Combined with ZF-3 (no module gate), this means:

- Cashier role can provision OCR vendor API keys (`PATCH /zf/settings`).
- Cashier role can close reconciliation periods and post journals.
- Cashier role can approve OCR drafts and post inventory.

`PermissionBoundary` doesn't help here because no boundary is invoked.

Files: `backend/app/Http/Controllers/Zf/ZfController.php` (entire)

---

### ZF-7 · MEDIUM · `learnedUpsert` is not actually an upsert

Endpoint name + comment promise upsert behaviour. Implementation:

```php
public function learnedUpsert(Request $request) {
    $data = $request->validate([
        'sku'        => ['required', 'string', 'max:64', 'exists:products,sku'],
        'signals'    => ['required', 'array'],
        'confidence' => ['required', 'numeric', 'between:0,100'],
    ]);
    $l = ZfLearned::create($data);                  // ← plain create
    return ApiResponse::created(['id' => $l->id]);
}
```

Calling `PATCH /zf/learned` (FE wire) or `POST /zf/learned` twice for the
same SKU produces two `zf_learned` rows. No uniqueness constraint at the
controller level; the migration would have to be checked to know whether
a DB unique exists, but the controller is doing `create()`, not
`updateOrCreate()` on a unique key.

The FE comment in `retail.api.js:223-224` already half-acknowledges this:
> `patchLearned: BE has POST /zf/learned (upsert) — the FE PATCH call resolves to that until per-row updates land.`

It is not an upsert. Calling it repeatedly accumulates duplicates.

Files: `backend/app/Http/Controllers/Zf/ZfController.php:256-265`

---

### ZF-8 · MEDIUM · `postZfJournal` silently skips when chart isn't provisioned

```php
if (! $revenueAccountId || ! $cogsAccountId) {
    // Chart not provisioned — skip silently, close still succeeds.
    return;
}
```

The recon row is then marked `status='closed'` with the variance retained.
The user sees "Closed" — but books don't reflect the variance.

For mature tenants this is fine. For fresh installs / sandbox / accounting-
disabled plans, the page shows a closed reconciliation with a non-zero
variance that never propagated to the GL. There's no warning, no banner,
no badge to indicate "GL post was skipped because chart not provisioned."

Files: `backend/app/Http/Controllers/Zf/ZfController.php:455-478`

---

### ZF-9 · LOW · `mock` OCR provider exposed on production

```php
'config.ocr_provider' => ['sometimes', 'nullable', Rule::in(['google', 'aws', 'mock'])],
```

```jsx
const PROVIDER_OPTIONS = [
  { value: 'mock',   label: 'Mock (no API key needed — demo data)' },
  { value: 'google', label: 'Google Document AI' },
  { value: 'aws',    label: 'AWS Textract' },
];
```

The mock provider is selectable on every tenant's settings page. There's no
"demo only" guard or env-restricted flag. A tenant on production can leave
the OCR provider set to `mock` and still see Done jobs and approved receivings
— the data is fake.

Useful during sales demos. Risky if a real tenant uses it accidentally.

Files: `backend/app/Http/Controllers/Zf/ZfController.php:649`,
`app/src/modules/retail/zf/SettingsPage.jsx:66-70`

---

### ZF-10 · LOW · Misleading FE comment

```js
// /zf/ocr stays demo (no backend; FE OcrPage renders a DemoBanner).
```

This comment is wrong. `OcrPage.jsx` is fully wired against `/zf/jobs` with
`kind='ocr'`. `ZfOcrService::process` runs end-to-end (vendor → match → draft
receiving). There is no `DemoBanner` anywhere in `OcrPage.jsx`.

A developer reading the comment will skip wiring something already wired.

Files: `app/src/modules/retail/retail.api.js:222`

---

### ZF-11 · LOW · No idempotency or cancellation on `jobsRun`

`jobsRun` rejects with 422 when status is `running` or `done`. There is no:

- retry path for a `failed` job (must `POST /zf/jobs` again, losing the audit trail)
- cancel endpoint for a stuck `running` job (the row needs a DB update)
- timeout / heartbeat so a crashed worker can be detected

Operational impact is small until OCR fails at scale. Worth knowing now.

Files: `backend/app/Http/Controllers/Zf/ZfController.php:97-144`

---

## 4. What works well

To stay honest, the ZF surface has real strengths:

- **`ZfOcrService` is the only non-stub kind and it's done well.** Real
  Google / AWS provider switch, real line-matching against the SKU catalogue,
  real `learned_from_sku` confidence scoring, real draft → posted receiving
  flow with auditor approval.
- **Reconciliation auto-synthesis from receivings + POS** (`synthesiseFromReceivingsAndPos`,
  lines 517-628) is well-thought-out — a fresh tenant gets a non-empty page
  on day 1, derived from real data.
- **Tender-brand breakdown is normalised** to the actual POS total
  (lines 602-613) rather than the raw payment sum, which avoids the
  "payments don't equal sale total" data-quality issue that the same code
  flags in a comment.
- **`OcrPage.jsx` + `ZfOcrReviewModal.jsx`** correctly drive the
  "OCR → review → approve → post inventory" flow with auditor sign-off.
- **`BelongsToTenant` global scope** is consistently applied to every ZF
  model, so the missing tenant filters in `ZfController` (e.g.
  `ZfJob::orderByDesc('created_at')` without an explicit `where`) don't leak
  across tenants — the scope catches it.

---

## 5. Recommended fix order (when ZF work resumes)

These are listed for sequencing only; this branch makes no changes.

1. **ZF-1 + ZF-2** together — gate the stubs behind an explicit
   `'NOT_IMPLEMENTED'` `ApiException`, and remove the catch-all that
   converts failures into 200s. One small refactor in `jobsRun`.
2. **ZF-4** — drop the `actual` field from the close request, or guard
   it behind `is_null($reconciliation->actual)`. Either way, do not
   trust the request body when posting GL.
3. **ZF-5** — wrap `/zf/jobs/{id}/ocr-approve` and
   `/zf/reconciliation/{id}/post` with the same step-up middleware
   already used for returns and journals.
4. **ZF-6 + ZF-3** — add `authorizeP('zf.*')` permissions to each method;
   wrap the route group in `module:zf` and seed plan grants accordingly.
5. **ZF-7** — change `create()` to `updateOrCreate(['company_id' => $cid, 'sku' => $sku], …)`.
6. **ZF-8** — return a banner field in `reconciliationPost`'s response
   when the chart skip happened, so the FE can show a warning.
7. **ZF-1 stubs (real implementations)** — wire `reconcile`, `shelf_scan`,
   `zatca_submit` to actual services. Until then, validation should reject
   those `kind` values for tenants that don't have the feature enabled.

None of these are deep refactors; the longest is a half-day of work.

---

**End of ZF audit. No code changes proposed in this branch.**