# Audit findings register — KeluargaMart Monorepo

Audited commit: `288b6cea77c3aa8c77e314f0bbef48d7bf35828e` (branch `main`, 2026-10-18)
Audit date: 2026-10-18
Companion document: `docs/KeluargaMart-Monorepo-Complete-Documentation.docx` (chapter 38)

Severity criteria:

| Severity | Criterion |
|---|---|
| Critical | Data loss, financial corruption or full authorisation bypass in normal operation. |
| High | Breaks a quality gate or destroys data under a plausible mistake; or a needed security control is absent. |
| Medium | Wrong behaviour or documentation on a path real users follow, with a workaround. |
| Low | Hygiene, drift, or a latent trap that does not affect today's operation. |
| Informational | Worth recording; no action required now. |

---

## F-01 — Legacy Laravel framework tables ship in the schema and are never used

- **Severity:** Low · **Area:** Database
- **Evidence:** `apps/backend/migrations/000001_init_schema.sql` creates `cache`, `cache_locks`, `jobs`, `job_batches`, `failed_jobs`, `sessions`, `migrations`. The Go backend replaces all of them with Redis or bearer tokens (`pkg/redis`, `internal/auth/token.go`). Verified by `pg_stat_user_tables` (0 rows, 0 code references outside the migration).
- **Impact:** Dead objects invite confusion during incident response — someone will look for queued jobs that never exist.
- **Recommendation:** Drop them in a dedicated migration after confirming no external tool reads them.
- **Verification:** `grep -rn "FROM jobs\|INTO jobs" apps/backend` returns nothing.
## F-02 — Documentation points at module paths that do not exist

- **Severity:** Medium · **Area:** Documentation
- **Evidence:** `AGENTS.md:20-21` and `AGENTS.md:72` cite `internal/database/migrations` and `internal/modules/rbac/catalog.go`. The real paths are `apps/backend/migrations` and `apps/backend/internal/auth/catalog.go`. `docs/architecture/overview.md` repeats both.
- **Impact:** A contributor or agent following the canonical instruction file lands on nonexistent directories and may create a second convention.
- **Recommendation:** Correct the paths in `AGENTS.md`, `docs/architecture/overview.md` and `apps/backend/AGENTS.md`.
- **Verification:** `test -d apps/backend/internal/database` → false.

## F-03 — CI pins Go 1.24 while go.mod requires 1.26.5

- **Severity:** High · **Area:** CI/CD
- **Evidence:** `.github/workflows/ci.yml:41` sets `go-version: '1.24'`; `apps/backend/go.mod:3` declares `go 1.26.5`.
- **Impact:** The backend job cannot build a module requiring a newer toolchain; the quality gate fails before any test runs.
- **Recommendation:** Use `go-version-file: apps/backend/go.mod` in both jobs.
- **Verification:** CI run on the backend job.

## F-04 — Root `test` script mixes incompatible runners

- **Severity:** Medium · **Area:** Tooling
- **Evidence:** `package.json` → `"test": "turbo run test && pnpm test:backend"`. Turbo runs the JS suites; the Go step then runs from the root without a database, so `internal/integration` skips and the remaining packages rely on `DATABASE_URL` being unset.
- **Impact:** `pnpm test` reports success while the integration suite never ran.
- **Recommendation:** Split into `test:unit` / `test:integration` and document the DB requirement in `CONTRIBUTING.md`.

## F-05 — Integration tests TRUNCATE 66 tables when consent is given

- **Severity:** High · **Area:** Testing
- **Evidence:** `internal/integration/helpers_test.go` (`requireDestructiveConsent`, `setUp`) truncates `users`, `customers`, `products`, `sales_orders` with CASCADE. Replaying every migration into a scratch database and running `TRUNCATE users … CASCADE` produced **66 cascade notices** — 67 tables emptied including `users` itself (measured).
- **Impact:** One exported `DATABASE_URL` plus `KG_ALLOW_DESTRUCTIVE_DB_TESTS=1` against a shared database destroys business data. The file's own comment records this happening once already.
- **Recommendation:** Additionally refuse to run unless the target database name matches a throwaway pattern (e.g. `*_test`).

## F-06 — Admin/owner permission map serialises as a wildcard only

- **Severity:** Low · **Area:** Auth
- **Evidence:** `GET /api/v1/auth/me` returned `{"*":true,"sales.lhpp.merge":true,"sales.lhpp.view":true}` for the seeded admin. `EffectivePermissions` publishes `*` for admin/owner (`internal/auth/model.go`).
- **Impact:** A client testing a concrete key rather than the wildcard sees an empty grant set. ERP has an explicit fallback and POS a special case; a new client would regress.
- **Recommendation:** Publish the expanded key set alongside the wildcard, or document the wildcard contract in `packages/permissions`.

## F-07 — `SMTP_FROM` is documented but never read

- **Severity:** Medium · **Area:** Configuration
- **Evidence:** `.env.example` declares `SMTP_FROM`; `pkg/config.Config` has `SMTPFrom`; `config.Load()` never calls `getEnv("SMTP_FROM", …)`. `mailer.SMTPMailer` therefore sends an empty `From`.
- **Impact:** Password-reset mail from a real relay is rejected or spam-filed, breaking account recovery.
- **Recommendation:** `SMTPFrom: getEnv("SMTP_FROM", "no-reply@keluargamart.local")`.

## F-08 — Generated role list drifts from the backend

- **Severity:** Medium · **Area:** Auth
- **Evidence:** `packages/permissions/src/index.js` `ROLES` = admin, owner, finance, warehouse, cashier, customer, store_manager, system (8). `internal/auth/model.go` `AllRoles` and migration `000057` seed 16 roles; `customer` is not a backend role, while gm/bm/spv/sales/buyer/finance_head/hr_manager/hr_staff/employee are missing from the client list.
- **Impact:** Client-side role checks are wrong for nine real roles and offer a nonexistent one.
- **Recommendation:** Generate `ROLES` from `AllRoles` in `scripts/gen-permissions.py`.

## F-09 — Migration 000058 only applies inside one transaction

- **Severity:** Low · **Area:** Database
- **Evidence:** `000058_expand_role_permissions_wildcards.sql` creates `TEMP TABLE catalog_spec … ON COMMIT DROP`. Applied statement-by-statement, the table is dropped before the `INSERT` and the migration fails with `relation "catalog_spec" does not exist` (reproduced).
- **Impact:** Out-of-band runs (`psql -f`, GUI clients) fail and leave the schema half-migrated.
- **Recommendation:** Replace the temp table with a `VALUES` CTE, or document `--single-transaction` as mandatory.

## F-10 — Two compose stacks disagree on PostgreSQL version and ports

- **Severity:** Medium · **Area:** Deployment
- **Evidence:** `infra/containers/compose.prod.yml` uses `postgres:16-alpine`; `apps/backend/docker-compose.yml` uses `postgres:17-alpine`. `deploy/.env.prod.example` maps `ERP_PORT=3003` / `POS_PORT=3005`, while `compose.prod.yml` defaults 3001/3003 and `docs/deployment/cloudflare-tunnel.md` fixes 3001/3002/3003/3004.
- **Impact:** Following the wrong runbook yields a stack whose ports do not match the tunnel ingress, on a different database major version.
- **Recommendation:** Make `infra/containers/compose.prod.yml` the only stack; retire or clearly mark the others.

## F-11 — nginx entrypoint hard-fails when resolv.conf is absent

- **Severity:** Low · **Area:** Deployment
- **Evidence:** `apps/erp/deploy/nginx-entrypoint.sh` exits non-zero when `/etc/resolv.conf` has no nameserver.
- **Impact:** In a runtime whose resolv.conf is generated after the entrypoint, the container exits instead of degrading.
- **Recommendation:** Document the dependency, or fall back to a literal upstream.

## F-12 — CORS defaults to localhost and permits a wildcard

- **Severity:** Medium · **Area:** Security
- **Evidence:** `pkg/config` default `CORS_ORIGINS=http://localhost:3000`; `pkg/middleware/cors.go` supports `*` (non-credentialed). `compose.prod.yml` does not set `CORS_ORIGINS`.
- **Impact:** A production deployment keeps the localhost default unless the operator adds it; if `*` is chosen, any origin may call the API with a token it already holds.
- **Recommendation:** Set the four public hostnames explicitly per environment.

## F-13 — Unknown `X-Request-Id` values are silently replaced

- **Severity:** Informational · **Area:** Observability
- **Evidence:** `pkg/middleware/requestid.go` (`isValidRequestID`).
- **Impact:** Client correlation IDs disappear without a signal.
- **Recommendation:** Log at debug when a supplied ID is discarded.

## F-14 — ERP mints a random Idempotency-Key for unmigrated writes

- **Severity:** Medium · **Area:** Financial correctness
- **Evidence:** `apps/erp/src/services/http.js` injects `idem-fallback-<ts>-<rand>` for any POST/PUT/PATCH without a caller-supplied key, and its comment states this makes the server's dedup branch unreachable.
- **Impact:** A double-clicked money form still posts twice; the fallback hides which call sites are unprotected.
- **Recommendation:** Throw in development and treat the warning as a worklist.

## F-15 — Report templates vanish under a cascade truncate

- **Severity:** Low · **Area:** Reports
- **Evidence:** `report_templates` is seeded by migrations 000064-000088 and has an FK to `users(created_by)`. A `TRUNCATE users … CASCADE` reached it (observed). A migrate-only database has 22 rows; after the cascade, 0.
- **Impact:** PDF rendering returns 404 `Active template not found` with no hint about the cause.
- **Recommendation:** Surface template counts in a health detail.

## F-16 — Cloudflare ingress is documented but not verifiable from the repo

- **Severity:** Informational · **Area:** Deployment
- **Evidence:** `docs/deployment/cloudflare-tunnel.md`; no `config.yml` is committed (it carries the tunnel UUID/credentials).
- **Impact:** A port change in compose silently breaks a hostname.
- **Recommendation:** Keep the mapping in one env file consumed by both compose and the runbook.

## F-17 — POS has no offline queue despite the documentation

- **Severity:** Medium · **Area:** Documentation
- **Evidence:** `docs/architecture/overview.md` and `apps/pos/AGENTS.md` describe an offline queue. No service worker, IndexedDB queue or pending-sync code exists under `apps/pos/src` (only localStorage caches for settings and session).
- **Impact:** A cashier may believe sales survive a network drop; they do not.
- **Recommendation:** Implement the queue or correct the documentation.

## F-18 — Local build output can be mistaken for the deployed artifact

- **Severity:** Informational · **Area:** Build hygiene
- **Evidence:** `apps/erp/dist`, `apps/pos/dist`, `apps/b2b/.output` exist in the working tree (gitignored); the Dockerfiles rebuild from source.
- **Impact:** A stale local build could be demoed as current.
- **Recommendation:** Treat GHCR images as the deployment source of truth.

## F-19 — sqlc schema snapshot lags the migration set

- **Severity:** Low · **Area:** Database tooling
- **Evidence:** `apps/backend/db/schema.sql` declares 86 tables; the migrated schema has 92. Missing: `report_templates`, `sales_targets`, `ticket_comments`, `tickets`, `warehouse_deposits`, `schema_migrations`. Those modules use hand-written SQL.
- **Impact:** `sqlc generate` cannot type-check queries against the newer tables.
- **Recommendation:** Refresh `db/schema.sql` and add the missing queries to `db/query`.

## F-20 — Generated documentation has no regeneration job

- **Severity:** Informational · **Area:** Documentation
- **Evidence:** `docs/docgen/build_doc.py`, `docs/audit/tools/build_inventories.py` and `docs/diagrams/gen_erd.py` reproduce this audit; `.github/workflows/ci.yml` does not run them.
- **Impact:** The document can silently drift.
- **Recommendation:** Add a scheduled job that regenerates and fails on a diff.

## F-21 — OpenAPI `RoleName` enum lists 8 roles, backend validates 16

- **Severity:** Medium · **Area:** API contract
- **Evidence:** `apps/backend/docs/openapi.yaml` `components.schemas.RoleName` = admin, sales, finance, warehouse, hr_manager, hr_staff, employee, system. `AllRoles` adds owner, gm, bm, spv, store_manager, cashier, buyer, finance_head.
- **Impact:** Typed clients generated from the contract cannot represent an outlet-level role.
- **Recommendation:** Generate the enum from `AllRoles` in the same script that writes `packages/permissions`.

## F-22 — Launch-reset migration wiped the seeded role matrix and nothing restored the bulk of it

- **Severity:** High · **Area:** Authorization
- **Evidence:** `apps/backend/migrations/000061_rbac_launch_reset.sql` runs `DELETE FROM role_permissions` and deliberately re-inserts nothing; `seed.sql` contains zero `role_permissions` rows. Measured on a database with all 123 migrations applied:
  - **860** rows before 000061 → **304** rows after.
  - **76 of the 201 catalogue keys are held by no role at all** (verified by expanding every row with the backend's own glob rule).
  - Unheld families include: all `masterdata.products.*` / `categories.*` / `units.*` / `brands.*` / `suppliers.*` writes, `masterdata.suppliers.view`, `finance.reports.view`, `system.users.*`, `system.roles.*`, `system.activity-logs.*`, `system.reports.*`, `system.stores.*`, `accounting.accounts.*`, `sales.sales-vouchers.*`, every `distribution.*` key, `sales_tracking.locations.view`.
- **Runtime confirmation:** created a `gm` user and logged in. `GET /api/v1/reports/profit-loss` → **403**, `GET /api/v1/stores` → **403**, `GET /api/v1/activity-logs` → **403**, `POST /api/v1/products` → **403**. `GET /api/v1/products` (public) → 200. A `store_manager` gets 403 on `/api/v1/users` and `/api/v1/reports/profit-loss`, 200 on `/api/v1/inventory/stock`.
- **Navigation impact:** of 66 permission-gated ERP navigation entries, **19 are hidden from every non-bypass role** (Supplier, Voucher Penjualan, all financial reports, COA, delivery orders, drivers, tracking map, users, roles, report templates, activity log). A GM sees 16 of 66 items.
- **Impact:** On a fresh deployment only admin and owner can create a product, read financial reports, administer users or deliver an order. This is recoverable (an admin can re-grant through `PUT /role-permissions`) but it is not the documented default, and nothing in the repository says the matrix starts empty by design for these keys.
- **Recommendation:** Re-seed the intended baseline matrix in a migration (or in `seed.sql`), and add the coverage test described in F-23.
- **Verification:** `SELECT count(*) FROM role_permissions;` → 304; the per-key holder query is in `docs/audit/tools/README.md`.

## F-23 — No automated check that a permission key is actually reachable

- **Severity:** Medium · **Area:** Authorization
- **Evidence:** `cmd/server/routes_contract_test.go` proves each route *has* a gate; `scripts/check-drift.py` proves the catalogue and route list agree. Nothing asserts that a key is granted to some role.
- **Impact:** A permission can be introduced, wired to a route and documented while being unusable by anyone but admin/owner — exactly the state F-22 describes. The gap is invisible until a user reports a 403.
- **Recommendation:** Add a test that expands `role_permissions` and fails when a catalogue key has no holder, unless the key is on an explicit reviewed allowlist.

## F-24 — Two live routes bypass the route/OpenAPI contract entirely

- **Severity:** Medium · **Area:** API contract
- **Evidence:** `cmd/server/main.go:1784-1785` mounts `/api/v1/reports/render` and `/api/v1/reports/preview` with `rr.HandleFunc`, not `rr.Get/Post`. `parseGatedRoutes` in `cmd/server/routes_contract_test.go` enumerates only Get/Post/Put/Patch/Delete, and `scripts/check-drift.py` reads only `openapi.yaml` — so these paths appear in neither the 373-operation count nor the contract.
- **Impact:** The contract says 373 operations; the router serves 375. A client generated from `openapi.yaml` cannot see the render endpoint that every ERP and POS print button calls, and a future gate regression on it would not be caught by the route table.
- **Runtime confirmation:** `GET /api/v1/reports/render?template=pos-receipt&format=pdf&sales_order_id=1` returned a 25 869-byte PDF while the path is absent from `openapi.yaml`.
- **Recommendation:** Document both paths, mount them with explicit methods, and extend the contract test to include `HandleFunc` registrations.

---

## Not a defect, but worth knowing

- The backend deliberately boots without a database (health-only mode) and without Redis (degraded readiness). This is documented behaviour, not a failure.
- `ALLOW_NEGATIVE_STOCK` defaults to `true`. That is a business decision (backorder support), not a bug — but `compose.prod.yml` does not pass the production value from `deploy/.env.prod.example`.
- `packages/ui` and `packages/api-client` are declared but unused. No runtime impact; a maintenance cost.
- No RLS exists. Tenancy is enforced in the service layer; this is a deliberate trade, recorded as a residual risk rather than a defect.