Knowledge-transfer tracker

Why this page exists

The rest of these docs contain no open questions — every claim is traced to code or confirmed against staging. Two kinds of thing don’t belong inline in engineer-facing docs, so they live here instead:

1. Known code bugs — real defects found while writing the docs, verified against source. Each is safe to turn into a ticket as-is.
2. Environment unknowns — facts that genuinely cannot be read from the repo (prod infra endpoints, org URLs, on-call owners). These are for the Robusta→ODH handover session to fill in, not gaps in the documentation.

When one of these is resolved, fix it at the source (file the bug / fill in the endpoint) and delete the row — this page should trend toward empty.

Known code bugs (verified against source)

These were found and confirmed against the current develop code. File them; don’t work around them silently in the docs.

Middleware “Unsupported Country” returns 500, not 404 bug · file a ticket

Where: crms-middleware/app/Exceptions/UnsupportedCountryException.php

What: The class extends a plain \Exception and calls parent::__construct('Unsupported Country', Response::HTTP_NOT_FOUND) — but the second constructor argument is the exception code, not an HTTP status. There is no render() and it is not an HttpException, so Laravel 9 renders it as a generic HTTP 500 (“Server Error”). Any doc or client that expects a 404 for an unknown country is wrong: today it’s a 500.

Impact: A support engineer triaging an unknown-country failure looks for a 404 that never comes; the real signal is a 500 with a generic body. Affects both the broker-lead path (rare — country is derived from a validated destination) and the sales-man path (more likely — the country rides on a user-supplied header).

Suggested fix: Give the exception a render() that returns response()->json(['message' => 'Unsupported Country'], Response::HTTP_NOT_FOUND), or make it extend Symfony\Component\HttpKernel\Exception\NotFoundHttpException. Then update the reference status table and the runbook back to 404.

Montenegro RESERVED = 7 is missing from the status map bug · file a ticket

Where: the Montenegro sale-status mapper (note the class filename is itself misspelled Monetengro…).

What: Egypt/Oman map 100000003 → RESERVED; Montenegro’s getReservedStatus() map does not contain its RESERVED = 7 code, so a Montenegro sale in that state maps to null and shows a blank/wrong reserved status in the UI.

Impact: Montenegro sales can display no status even though the CRM has them reserved — a mapping gap, not a delivery failure. Covered in the Sales-Man flow.

Suggested fix: Add the 7 → RESERVED entry to the Montenegro map; fix the Monetengro filename in the same change.

email on the lead form has no email-format rule quirk · confirm intent

Where: SubmitLeadRequest::rules() (backend).

What: email is nullable, string with no email rule, so any string is accepted and forwarded to Dynamics. interested_in_unit_id is neither required nor nullable, so a present-but-null value fails the string rule. Documented as real quirks on the reference page.

Impact: Garbage emails reach the CRM. Low urgency, but confirm it’s intentional before “fixing” it — the other two personas’ request classes may rely on the same loose contract.

Local audit row is skipped on a 201 (loose == 200) quirk · confirm intent

Where: LeadController@submitLead (backend).

What: The user_requests audit row is written only on an exact == 200. If the CRM ever returns 201 Created, the lead is created but no local row exists — so “no local row” does not strictly prove “not created”. Called out in the runbook decision tree.

Impact: Reconciliation edge case. Confirm whether the CRM can return 201 on this path; if so, broaden the check to 2xx.

unit_sale.crm_status is inconsistently typed P2 · data integrity

Where: the unit_sale.crm_status column, written from several paths.

What: crm_status is stored as different types depending on the write path: the sale webhook validates it as an integer (UpdateSaleRequest.php:13) and persists the raw CRM integer (UpdateSalePortal.php:20), whereas other paths write literal strings — 'contracted' (CreateLeadSalesInvoice.php:47), $sale['sale_status_id'] ?? '' (TransformSalesCrm.php:62) — and some queries compare it as 'draft'/'canceled' (RouteServiceProvider.php:78, AccessCardController.php:29). (The strtolower(...contracting_status) in ResponseMapper/Shopper/UpdateSale.php is an outbound response field, not a DB write.)

Impact: A query that filters crm_status by a string will silently miss rows written as integers, and vice-versa — an easy source of “the sale is there but doesn’t show in this list” bugs. The local status (UnitSaleStatus) column is the reliable one.

Suggested fix: normalize crm_status to a single type at every write site (cast to UnitSaleStatus or keep the raw CRM integer, but not both), and align the queries.

Payment, PDF & EOI security backlog (verified against source)

These were found while documenting the payment pipeline, the sales-offer PDF service, and the EOI flow. Every row is confirmed against current code (file:line). The flow pages carry the load-bearing cautions in prose; this is the file-a-ticket list. Severity is the author’s read — confirm with the security owner before prioritising.

Payment pipeline

Sev	Issue	Where	Fix
P0	Paymob HMAC verification is commented out — the webhook accepts any payload	PaymobService.php:146-152 (checker exists but unused, :166-211)	Re-enable checkTransactionCallbackHMAC, wire it into handleTransactionCallback, reject on mismatch
P0	Thawani & WSPG callbacks do no verification — trust the URL {status} segment + echoed payment_reference_id	CallbackController.php:44-65; no verify in ThawaniService/WSPGService	Server-confirm the session / recompute the return signature before marking success
P0	Charged amount is client-supplied, not bound to the unit’s reservation fee	PaymentRequest.php:13-18 → PaymentSale.php/services	Derive/validate the charge server-side from the unit price; ignore the client amount
P1	Callback routes are public/unauthenticated (only FormatApiResponse stripped)	routes/api.php:31-38	Gate by gateway source-IP/shared-secret; rely on signature verification
P1	No idempotency/replay guard — a re-delivered success re-submits to the CRM + re-fires emails	PaymentService.php:36-73	Short-circuit if the row is already success (dedupe on payment_reference_id)
P1	Operator-precedence bug if (! $response->getStatusCode() == 200) — parses as (!status)==200, so the failure branch never throws	PaymobService.php:69, ThawaniService.php:52	Str::startsWith((string)$status,'2') (as the fixed sites do) or $status !== 200
P1	== instead of hash_equals for HMAC compare (timing; currently dead code)	PaymobService.php:210	hash_equals(...)
P2	sleep(10) blocks the browser-return thread	PaymentService.php:102	Remove; poll/webhook-drive readiness (shared with EOI)
P2	Faker in production — client_reference_id sends the literal "Faker\Core\Uuid"	ThawaniService.php:11,29	Send Str::uuid()->toString(), not Uuid::class
P2	Null-deref on unknown reference id (fail path)	PaymentService.php:81-97	Guard/early-return when $transaction is null
P2	Null-deref in Paymob browser callback	PaymentService.php:103-104	Null-check the $data['order'] lookup before ->model

Sales-offer PDF service

Sev	Issue	Where	Fix
P1	SSRF — unrestricted fetch(imageUrl) on caller-supplied URLs (reachable via unit.masterplan_image, masterplans[].image, gallery[])	backend-pdf-service/src/pdf/embed-image.ts:6-33	Scheme/host allowlist + private-IP/metadata block + response size cap before buffering
P2	Image-fetch failure is console.error’d to the daily log but then swallowed to null — silent to the caller and the PDF output (the image is just dropped)	embed-image.ts:29-32	Surface the failure to the caller/output, not only the log
P2	cors() with no origin allowlist	backend-pdf-service/src/index.ts:17	Restrict allowed origins to the backend host(s)
P2	Dead synchronous route still wired — getSalesOfferPdf no longer exists, so the route throws	routes/api/broker.php:132-134, sales-man.php:79-81	Delete the two route lines (shopper has none)
P2	Puppeteer runs --no-sandbox (compounds the SSRF surface + page.setContent)	capture-render.ts:94-112	Harden the container/egress as part of the SSRF fix
P3	Non-constant-time token compare (===)	sales-offer.ts:13-15	crypto.timingSafeEqual (low risk on an internal network)

EOI flow

Sev	Issue	Where	Fix
P1	Broker require-action uses a hardcoded fallback salesman source id (a literal GUID)	UpdateEOIWithRequireAction.php:44	Resolve the real source id; drop the magic constant
P1	Shopper show calls the CRM as a hardcoded service-account identity (a literal crmadmin@… UPN) on a public, token-gated endpoint	Shopper/EOIController.php:32	Move the identity to config/env and scope it down
P2	resendPaymentLink is dead code (// No more be used) but still route-reachable	Broker/EOIController.php:373-392	Remove the endpoint + route, or restore it as a real recovery path

(Paymob HMAC + sleep(10) also affect EOI online payments — see the Payment-pipeline rows above.)

CRM credential isolation

Where: crms-middleware/config/services.php groups link_dev_auth (:165-171) and egypt/oman/montenegro (:35-70). P2 · hardening

What: The write path (Link-Dev → Azure Logic Apps) and all three read paths (per-country direct Dynamics OData) authenticate with the same Azure AD app registration — the client_id and client_secret are byte-identical across LINK_DEV_* and all three *_CRM_* groups, on one tenant (verified by equality over the local .env; no values printed). The only differences are the token endpoint (v1.0 resource= vs v2.0 scope=), the cache key, and the target host.

Impact: One app-registration secret is the master key to read every org’s data and drive every Logic-App write. There is no per-country and no per-direction credential isolation, so rotating or leaking that single secret affects all three CRMs and the write gateway at once — a large blast radius and a least-privilege gap. Documented in full on the CRM integration model.

Suggested fix: split into separate app registrations per direction (and ideally per country) with least-privilege scopes; at minimum, use distinct secrets so a rotation/leak is contained.

Webhook routes don’t enforce their scope

Where: backend/routes/api/crms-middleware.php:7. P3 · hardening

What: The middleware→backend webhook routes apply the bare client guard (CheckClientCredentials), which accepts any valid Passport client token regardless of scope. The token the middleware mints carries scope=middleware-webhook (CrmsMiddleware/AuthController.php:14-19), but nothing on these routes checks it — so the scope is carried, not enforced. (By contrast, the middleware’s own portals-api and crm-webhook routes use client:<scope> and do enforce.)

Impact: A least-privilege gap — any service holding a valid client-credentials token for this Passport server could call the inbound webhooks, not only a middleware-webhook-scoped one. Low severity on a closed internal network, but it means the scope provides no actual authorization here.

Suggested fix: gate the webhook routes with client:middleware-webhook instead of the bare client alias.

Frontend OAuth secret + Currency-Layer key ship in the browser bundle

Where: frontend/.env.example — NX_CLIENT_SECRET (:31) and NX_CURRENY_LAYER_API_KEY (:36).

P1 · secret exposure

What: Vite inlines every NX_* / VITE_* var into the shipped browser bundle (frontend/CLAUDE.md §2.4). Two of them are genuine secrets: an OAuth client secret and the Currency-Layer API key. Anything in an NX_* var is readable by every visitor who opens the bundle.

Impact: These secrets are effectively already public — extractable from the production JS. The OAuth client secret client-side undermines the confidential-client assumption; the Currency-Layer key can be abused/quota-drained. (By contrast NX_FIREBASE_API_KEY and the NX_SENTRY_*_DSN are public-by-design and fine.)

Suggested fix: rotate both immediately, and move the OAuth client-secret exchange + the Currency-Layer call server-side (a backend proxy endpoint) so no secret is inlined into the SPA.

Environment unknowns (confirm during handover)

Not defects — these simply cannot be read from the repo. Fill them in with the Robusta handover and your ops lead, then wire them into the runbook’s “Access & escalation” table.

Unknown	Why it’s not in the repo	Who confirms
Production deploy mechanism	The architecture topology shows GitLab → CodePipeline → CodeDeploy → EC2 from Robusta’s handover docs, drawn dashed/unverified. Whether prod is CI-pipeline or on-box pull isn’t in the code.	Robusta / ODH DevOps
Log sink URL (staging + prod)	LOG_CHANNEL/LOG_LEVEL are in code; where logs ship is env config.	Ops lead
Sentry project URL	Backend + frontend report to Sentry; the project/DSN is an env secret. Middleware is not in Sentry at all.	Ops lead
Tempo / OTLP collector endpoint	OpenTelemetry is backend-only (OTEL_SERVICE_NAME=orascom-backend); the repo only shows a local-dev collector.	Ops lead
Per-country Dynamics org base URLs	The read path hits {orgBaseUrl}/api/data/v9.x/…; the three org URLs are secrets.	CRM team (Link Dev)
Team owners / Slack / on-call	The runbook owner table lists fault domains, not the concrete people.	Eng manager

Documentation hygiene notes

Minor items tracked so they don’t silently rot.

Note	Status
Dashboard screenshot PII — broker-dashboard.png “Upcoming Meetings” showed lead names. Assessed as staging test fixtures (the account is the test broker “Broker Agent Egypt”; names follow synthetic patterns). The two personal names were blurred as a precaution.	resolved · blurred Confirm no real customer data exists on staging before any re-capture.
Stale PDF-service CLAUDE.md §2 rule 6 — claims “unset API_TOKEN ⇒ no auth”, but the code fails closed (401).	doc bug Correct the service doc to match sales-offer.ts:8-16.
PDF_SERVICE_URL default drift — config/services.php:69 defaults to http://localhost:3001; backend CLAUDE.md says http://pdf-service:3001.	doc bug Reconcile the doc with the code default.