Files
marketplaces/docs/superpowers/specs/2026-08-21-fork-harvest-design.md
sdarbinyan fd3ca85929 docs: fork analysis + improvement harvest spec and todo
Deep analysis of marketplaces-main.zip (hub.numus.cc/numus/marketplaces).

Findings:
- Not a fork of us. Separate platform monorepo (NestJS + Postgres +
  2 Angular apps + infra) sharing an older dexarmarket ancestor.
- Our repo is vendored inside it as reference/parallel-frontend/,
  SHA-256 pinned, dated 11 Aug 2026, classified "reference, not production".
- Zero VK/Yandex/OAuth code anywhere in their source. Their only
  customer login is Telegram, proxied to an external service.
- They lead on backend truth and ops; we lead on frontend depth,
  tests, e2e, and framework currency.

Three of their audit findings are still live in our code and are
defects, not just posture: plaintext ip-api.com call from an HTTPS
origin (mixed content, region detect silently dead), unvalidated
bypassSecurityTrustResourceUrl on a bank URL rendered in an iframe,
and provider credentials plus a partner ID literal in the bundle.

Adds:
- docs/FORK-ANALYSIS-2026-08-21.md - full comparison, their audit of
  us assessed line by line, and a VK ID + Yandex ID design.
- docs/superpowers/specs/2026-08-21-fork-harvest-design.md - working
  brief, five lanes, four waves, scope and rejection rules.
- docs/FORK-HARVEST-TODO.md - 42 items with effort, dependencies and
  acceptance criteria. Improvements only; nothing regresses our
  Angular version, test count, or architecture governance.

No implementation changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-21 09:31:54 +04:00

202 lines
14 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Fork Harvest — Design / Working Description
**Branch:** `improvements/fork-harvest` (cut from `B2B` @ `92f1c88`)
**Date:** 2026-08-21
**Input:** [FORK-ANALYSIS-2026-08-21.md](../../FORK-ANALYSIS-2026-08-21.md)
**Companion:** [FORK-HARVEST-TODO.md](../../FORK-HARVEST-TODO.md)
---
## 1. Purpose
Take **only the improvements** from the `hub.numus.cc/numus/marketplaces` archive. Nothing else. No architecture adoption, no code copying, no rewrite, no framework regression.
The archive is a competing platform monorepo, not a fork of us. It contains our repo verbatim as `reference/parallel-frontend/`. It is ahead of us on backend truth and operations, behind us on frontend depth, testing, and framework currency.
This document is the working brief for whoever executes the harvest — including a future session of me. It states what is true today in this repo, what changes, and how each item is proven done.
---
## 2. Constraint that shapes everything
**This repo has no backend.** 530 `.ts` files, Angular 22, zero server code. Our backend exists only as 17 contract documents in `docs/backend/`, implemented by another team.
That splits every harvested improvement into one of five lanes:
| Lane | Meaning | Where it lands |
|---|---|---|
| **A — Frontend** | We write the code, this sprint | `src/`, `e2e/`, `angular.json`, CI |
| **B — Contracts** | We write the requirement; backend implements | `docs/backend/*.md` |
| **C — Packages** | Ships in `@marketplaces/auth` (external repo `vitanovaPackages`) | package repo + DI wiring here |
| **D — Infra/Ops** | Deploy scripts and runbook | `scripts/deploy/`, `docs/DEPLOYMENT.md` |
| **E — Process** | Governance, gates, policy | `docs/backend/BACKEND-HANDOFF.md`, ADRs |
Anything that would require us to stand up Prisma, Postgres, or NestJS in *this* repo is out of scope. It becomes a Lane B contract line instead.
---
## 3. What we verified in our own code (2026-08-21, current HEAD)
Their audit was written against our 11 Aug snapshot. Re-checked against today's code:
| Their finding | Status now | Evidence |
|---|---|---|
| Admin session cookie set by JS, readable via `document.cookie` | **Already fixed** | zero `document.cookie` hits in `src/` |
| Admin JWT / refresh token in `localStorage` | **Already fixed** | no `setItem(*token*)` anywhere |
| `http://ip-api.com` from an HTTPS storefront | **STILL LIVE** | `src/app/services/location.service.ts:75` |
| Unconditional `bypassSecurityTrustResourceUrl` on a bank URL | **STILL LIVE** | `src/app/pages/cart/cart.component.ts:485` |
| Client-side `authorization-key` / `userid-value` headers | **STILL LIVE** | `src/app/services/api.service.ts:675` |
| Hardcoded partner ID in the bundle | **STILL LIVE** | `src/app/services/api.service.ts:143``'web-97ec-9c57-4dde-9037-3a68f7f83750'` |
| `localStorage` as persistence | **19 files**, mostly admin facades + editor draft storage | see TODO FH-A7 |
| Bundle 452 kB over a 700 kB budget | **Still true** | their measured build |
| Zero `idempot*` in the codebase | **Still true** | no Idempotency-Key sent on payment creation |
Two of those are live bugs, not just security posture:
- **`http://ip-api.com`** — browsers block mixed active content on an HTTPS origin. `detectLocation()` therefore always takes its error branch in production. Region auto-detect has been silently dead.
- **Bank URL in an iframe** — most acquirer 3-D Secure pages send `X-Frame-Options: DENY` / frame-ancestors CSP. The popup renders blank for those banks. Their spec calls this out explicitly: card checkout should navigate the current tab, not open an intermediate popup.
That reframes three of the "security" items as **defect fixes with a security benefit**, which is a much easier sell and a much better use of the sprint.
---
## 4. Selection rule — what counts as "an improvement"
An item is harvested only if it passes all four:
1. **It is better than what we have**, not merely different.
2. **It survives without their backend.** Either we can build it, or it is a contract line the backend team can implement against.
3. **It does not regress us.** Nothing that drops us to Angular 21, reintroduces mocks, or lowers our test bar.
4. **It is falsifiable.** There is a test, a check, or an observable state that proves it done.
Explicitly rejected by this rule (from the analysis §9): their Angular version, their `mock-data.service.ts`, their 25-test/zero-e2e posture, their env-pinned `ORDER_MANAGER_MARKETPLACE_SLUG`, their hardcoded server IP, their narrower 5-type section schema.
---
## 5. The harvest, by theme
### 5.1 Correctness primitives (the highest-value cluster)
Three patterns from their backend that are worth more than everything else combined, because each replaces application logic with a database guarantee:
**Conditional-UPDATE reservation.** One statement is their entire oversell defence:
```sql
UPDATE "MarketplaceInventory"
SET "reserved" = "reserved" + $qty
WHERE "marketplaceId" = $mp AND "variantId" = $variant
AND ("onHand" - "reserved") >= $qty
RETURNING "id"
```
Empty result set → `409`. No read-then-write window, no advisory lock, no retry loop. Goes into `PHASE-6-CART-CHECKOUT-CONTRACT.md` as a normative requirement, not a suggestion.
**Idempotency as a unique constraint.** `Payment.idempotencyKey UNIQUE` and `PaymentWebhookEvent @@unique([provider, eventKey])`. A duplicate insert throws, and the catch returns `{accepted: true, duplicate: true}`. Replay protection becomes structurally impossible to forget, versus an `if` somebody eventually deletes. Goes into `PHASE-7`.
**Append-only inventory journal.** Every stock change writes `reason`, `referenceType`, `referenceId`, `actorId`, resulting balance. This is the direct answer to the v3.1 plan's "we cannot explain your numbers" complaint — it makes every quantity reconstructible after the fact.
None of these are hard. All three are cheap to specify and expensive to retrofit.
### 5.2 Session and credential hygiene
Their model, which we adopt as the contract target: server-stored sessions, random 32 bytes, **stored as SHA-256 hash only**, HttpOnly + Secure + SameSite, revocable, one distinct cookie per contour (`bo_session` / `manager_session` / `marketplace_session`), Argon2id `memoryCost 65536 / timeCost 3 / parallelism 1`, mandatory TOTP with a signed 10-minute setup token, and password change revoking every live session in the same transaction.
Plus the twelve-line CSRF defence we do not have: a global `onRequest` hook rejecting any non-GET on an admin/manager path whose `Origin` is not in the configured allowlist.
Our side of this is subtractive: stop sending provider credentials from the browser, stop shipping a partner ID literal in the bundle.
### 5.3 Tenant and preview safety
Host → verified domain row → tenant, 30-second cache with explicit invalidation, `404` on unknown host with no fallback tenant. We have bootstrap-driven runtime config and no equivalent guarantee written down.
Their preview mechanism is the piece worth copying outright: an HMAC-signed token carrying `{marketplaceId, expiresAt, nonce}`, 15-minute TTL, delivered as a `storefront_preview` cookie, plus a global hook that returns `404 Preview mode is read-only` for any non-GET while that cookie is present. We have preview UI and no preview safety at all.
### 5.4 Publish, revisions, clone
`version = max(version) + 1`, immutable snapshot row, `publishedRevision` pointer flipped in the same transaction, rollback creates a *new* revision rather than rewriting history. Clone copies design and catalog assignments, **forces inventory to zero**, and never copies domains, customers, orders, or secrets; its category walk is topological with explicit cycle detection.
### 5.5 Product ideas worth taking
- **Order-manager as a fully separate contour** — separate URL, shell, cookie, login, scoped to one marketplace, with no visibility into catalog, design, domains, or payment settings. Removes an entire permissions surface rather than guarding it.
- **Digital goods in one table** — `FulfillmentMode: MANUAL | CODE_POOL`, a `DigitalCode` pool with `AVAILABLE/RESERVED/ASSIGNED/REVOKED`, encrypted values, `valueHash` unique per `(marketplace, variant)`, codes revealed only once the order is `PAID`. We have no digital-goods story; this is a complete one.
- **`DOMAIN_PENDING` as a real marketplace state**, not an error condition.
- **CSV marketplace import with `dryRun` default true** — bulk tenant creation as a first-class operation.
- **Their §22 acceptance list** as a ready-made e2e suite. Two of the fifteen are worth writing immediately: concurrent purchase of the last unit, and a replayed webhook.
### 5.6 Social identity — VK ID and Yandex ID
The archive has **zero** VK/Yandex/OAuth code; there is nothing to copy. What we take is the *session-issuing shape* of their Telegram flow and terminate VK/Yandex into it.
Design decisions, all of which belong in `@marketplaces/auth`:
1. **Provider-agnostic surface.** `SocialIdentityGateway` with a `SocialProvider` union, replacing today's VK-specific `VkIdGateway`. One controller pattern backend-side, one strategy object per provider.
2. **Backend-owned PKCE.** Our current interface passes `codeVerifier` from the client, which forces the browser to generate and hold the verifier. We are a confidential client. The backend generates `state` + `code_verifier`, stores them single-use for 10 minutes, and the browser only ever gets redirected. `completeCallback()` disappears from the frontend entirely.
3. **VK ID gotcha:** the callback returns `device_id` next to `code`, and the token exchange fails without it. This is the single most common VK ID integration bug and it must be in the contract text.
4. **Multi-tenant `redirect_uri` is a one-way door.** Both providers validate `redirect_uri` against an exact registered list; we cannot register one per tenant domain. Resolution: a single central identity host as the only registered callback, tenant carried inside the signed `state`, then a 302 back to the tenant domain with a short-lived signed handoff token the tenant API exchanges for its session cookie. **This must be decided before any code is written.**
5. **Identity conflict is not an upsert.** `@@unique([provider, providerUserId])` so the database refuses a silent rebind; conflicts route to controlled resolution.
Build order: provider-agnostic surface → VK ID → Yandex ID (a second strategy, roughly a day) → migrate Telegram onto `ExternalIdentity` → linking UI → email/phone OTP demoted to recovery.
### 5.7 Operations
Adopt: WAL archiving plus a *scheduled, proven* restore drill; a data network that is `internal: true` so "the database is not reachable from the internet" is structural rather than a firewall promise; host hardening we lack (fail2ban, sshd drop-in, sysctl).
Already better on our side, keep as-is: our `add-domain.sh` already pre-checks the DNS A record and runs `nginx -t` before and after; `server-setup.sh` already configures ufw. Their `provision-domain.sh` hardcodes the server IP — do not copy that shape.
### 5.8 Process
Their `DEVELOPER_HANDOFF.md` §7 is nine falsifiable invariants and is a better acceptance gate than anything currently in our delivery plan. Their PR policy (one functional area per PR; mandatory security impact and rollback plan; never touch payment/inventory/order state machines inside a redesign PR) and their release discipline ("a local build or the existence of a UI does not mean production readiness") are both worth adopting verbatim.
---
## 6. Sequencing
Four waves. Each wave is independently shippable; nothing in a later wave blocks an earlier one.
**Wave 1 — Defect fixes with a security benefit (this sprint, Lane A).**
The three live bugs: geo over HTTP, bank URL in an iframe, provider credentials and partner ID in the bundle. Plus `Idempotency-Key` on payment creation. All frontend, all provable, all things their audit will otherwise keep pointing at.
**Wave 2 — Contract hardening (Lane B, parallel with Wave 1).**
Write the correctness primitives, session model, tenant/preview rules, revision semantics, and inventory journal into `docs/backend/`. Costs no engineering capacity from the frontend team and immediately raises the bar the backend is built to.
**Wave 3 — Proof (Lane A).**
The two acceptance e2e tests, bundle budget as a blocking CI check, and the deployable split that gets us under budget.
**Wave 4 — Identity (Lane C).**
Blocked on the central-identity-host decision. Provider-agnostic surface, VK ID, Yandex ID, Telegram migration, linking UI.
Ops (Lane D) and process (Lane E) run continuously alongside.
---
## 7. Explicitly out of scope
- Merging the two codebases, in either direction.
- Reimplementing their backend here.
- Adopting their section schema, their template list, or their backoffice.
- Any dependency downgrade.
- Removing our boundary checker, cycle check, or coverage floor to match their looser governance.
---
## 8. Risks
| Risk | Mitigation |
|---|---|
| Contract lines in `docs/backend/` are written and never implemented | Pair each with an acceptance scenario in the handoff doc so it is a delivery gate, not a wish |
| The central-identity-host decision slips and blocks all of Wave 4 | It is the first item in the TODO; escalate on day one |
| Removing the client-side payment credential path breaks checkout before the server side exists | Confirm the server-priced checkout session path (already in `api.service.ts`) covers every live flow before deleting the legacy header path |
| The deployable split is larger than estimated | Wave 3 item, not a blocker for Waves 12; can ship the bundle-budget CI check first and let it fail loudly |
| Harvest is read as "they were right about everything" | The analysis records where they are behind us — tests, e2e, framework currency, frontend depth — and the TODO carries no item that regresses those |
---
## 9. Done means
- Every Wave 1 item has a test or an observable check proving it.
- Every Lane B item exists as normative text in `docs/backend/` with an acceptance scenario attached.
- The two §22 acceptance tests run in CI.
- Bundle budget is a blocking check and the storefront is under it.
- VK ID and Yandex ID both log a customer in through `@marketplaces/auth`, with the client never holding a secret, a token, or a code verifier.
- No item in this harvest lowered our Angular version, our test count, or our architecture governance.