Files
marketplaces/docs/FORK-HARVEST-TODO.md

232 lines
19 KiB
Markdown
Raw Normal View History

2026-08-21 09:31:54 +04:00
# Fork Harvest — TODO
**Branch:** `improvements/fork-harvest` (from `B2B` @ `92f1c88`)
**Design:** [2026-08-21-fork-harvest-design.md](superpowers/specs/2026-08-21-fork-harvest-design.md)
**Source analysis:** [FORK-ANALYSIS-2026-08-21.md](FORK-ANALYSIS-2026-08-21.md)
Improvements only. Nothing here regresses our Angular version, test count, or architecture governance.
**Effort:** S ≤ half a day · M ≤ 2 days · L > 2 days
**Lane:** A frontend · B backend contract · C `@marketplaces/auth` package · D infra/ops · E process
---
## Wave 0 — Decide first (blocks Wave 4)
- [ ] **FH-0.1 — Decide the central identity host** · L · Lane C · *blocker*
VK ID and Yandex ID both validate `redirect_uri` against an exact registered list. We cannot register one per tenant domain, and we cannot let tenants supply their own.
**Decision needed:** single central callback host (e.g. `id.<platform-domain>`) as the only registered URI, tenant carried inside signed `state`, 302 back to the tenant domain with a short-lived signed handoff token the tenant API exchanges for a session cookie.
**Also decide:** is one VK account across two of our storefronts one `Customer` or two? Their platform says two; our `Customer.marketplaceId` already implies two.
**Done when:** an ADR exists in `docs/context/adrs/` and both questions have a recorded answer.
- [ ] **FH-0.2 — Confirm the server-priced checkout path covers every live flow** · S · Lane A · *blocks FH-1.3*
`api.service.ts` already has a server-priced checkout session method. Confirm no production flow still depends on `createPayment(payload, headers)` before deleting the header path.
**Done when:** every caller of the legacy header path is enumerated and has a replacement.
---
## Wave 1 — Live defects with a security benefit (Lane A, this sprint)
- [ ] **FH-1.1 — Kill the plaintext third-party geo call** · S · Lane A
`src/app/services/location.service.ts:75` calls `http://ip-api.com/json/?fields=…` from an HTTPS origin. Browsers block mixed active content, so `detectLocation()` has been silently taking its error branch in production — region auto-detect is dead, not just insecure. It is also a third-party geo leak on every session.
**Do:** remove the direct call. Resolve region server-side (`GET /api/v1/geo/resolve`, backend reads the client IP) or drop auto-detect and keep the manual region picker.
**Done when:** zero `http://` literals in `src/`; a unit test asserts `detectLocation()` issues no cross-origin request to a non-allowlisted host.
- [ ] **FH-1.2 — Stop blindly trusting the bank redirect URL** · M · Lane A
`src/app/pages/cart/cart.component.ts:485``bypassSecurityTrustResourceUrl(bankUrl)` with no validation, rendered into a popup iframe. Most acquirer 3-D Secure pages send `X-Frame-Options: DENY`, so the popup is blank for those banks. Their spec: card checkout navigates the current tab, no intermediate popup.
**Do:** accept only an `https:` URL whose origin the backend returned in the payment response (backend allowlist, per their `safeHttpsUrl()`); navigate the current tab instead of framing.
**Done when:** a non-https or non-allowlisted URL is refused with a visible payment error; a test covers both the accepted and the refused case.
- [ ] **FH-1.3 — Remove provider credentials from the browser** · M · Lane A · *depends on FH-0.2*
`src/app/services/api.service.ts:675` sets `authorization-key` and `userid-value` headers client-side. `api.service.ts:143` ships a partner ID literal (`'web-97ec-9c57-4dde-9037-3a68f7f83750'`) in the bundle. Their audit's most serious finding, and it is correct.
**Do:** delete both header paths and the literal; the browser gets a checkout URL or a status endpoint, never a credential.
**Done when:** `grep -ri "authorization-key\|userid-value" src/` returns nothing; no partner ID literal in `dist/`; a CI check greps the built bundle for both patterns.
- [ ] **FH-1.4 — Send `Idempotency-Key` on payment creation** · S · Lane A
Zero `idempot*` anywhere in our codebase. Their API requires the header and rejects a key reused across a different order.
**Do:** generate one key per checkout attempt, stable across retries and across a double-click, sent on payment creation.
**Done when:** the existing `checkout-idempotent-click.spec.ts` asserts both requests carry the *same* key.
---
## Wave 2 — Contract hardening (Lane B, parallel with Wave 1)
Each item is normative text plus an acceptance scenario in `docs/backend/BACKEND-HANDOFF.md`, so it becomes a delivery gate rather than a wish.
- [ ] **FH-2.1 — Conditional-UPDATE stock reservation** · S · `PHASE-6-CART-CHECKOUT-CONTRACT.md`
`UPDATE … SET reserved = reserved + $qty WHERE (onHand - reserved) >= $qty RETURNING id`; empty result → `409`. Reservation TTL 15 min. Price read only from the server-side snapshot, never from the request.
**Acceptance:** two concurrent purchases of the last unit produce exactly one payable order.
- [ ] **FH-2.2 — Idempotency as unique constraints** · S · `PHASE-7-PAYMENTS-RECONCILIATION-CONTRACT.md`
`Payment.idempotencyKey UNIQUE`; a key reused against a different order/marketplace → `409`. `PaymentWebhookEvent @@unique([provider, eventKey])`; duplicate insert → `{accepted: true, duplicate: true}`. `eventKey` falls back to `sha256(rawBody)`. Signature verified against the **raw** body. Status poll as a 24-hour reconciliation fallback.
**Acceptance:** a replayed webhook neither completes the order twice nor moves stock twice.
- [ ] **FH-2.3 — Session and credential model** · M · `TRACK-S-SECURITY-RBAC-CONTRACT.md`
Server-stored sessions; random 32 bytes; **stored as SHA-256 hash only**; HttpOnly + Secure + SameSite; revocable; a distinct cookie per contour (`bo_session` / `manager_session` / `marketplace_session`). Argon2id `memoryCost 65536, timeCost 3, parallelism 1`. TOTP mandatory, gated by a signed 10-minute setup token. Password change ≥16 chars and revokes every live session in the same transaction. Role weights `ORDER_MANAGER 0 < VIEWER 1 < CONTENT_MANAGER 2 < ADMIN 3 < OWNER 4`, checked together with marketplace scope.
**Acceptance:** a CONTENT_MANAGER cannot read an unassigned marketplace through a direct API call.
- [ ] **FH-2.4 — Origin allowlist for admin mutations** · S · `TRACK-S-SECURITY-RBAC-CONTRACT.md`
Global hook: any non-GET on an admin/manager path whose `Origin` is not in the configured allowlist → `403`. CORS uses the same allowlist with `credentials: true`.
**Acceptance:** a cross-origin POST with a valid session cookie is refused.
- [ ] **FH-2.5 — Tenant by verified Host only** · S · `PHASE-9-TENANT-REGISTRY-DOMAINS-CONTRACT.md`
Normalize host (lowercase, strip trailing dot, strip port) → unique `hostname` row → require `verifiedAt` and `ACTIVE`. Short cache with explicit invalidation. Unknown host → `404`, never a fallback tenant. The public API never accepts a `marketplaceId` from the browser.
**Acceptance:** an unknown Host returns 404 and leaks no other tenant's data.
- [ ] **FH-2.6 — Signed preview token, read-only preview** · S · `PHASE-9-…`
HMAC-signed token carrying `{marketplaceId, expiresAt, nonce}`, 15-minute TTL, `storefront_preview` cookie. Global hook returns `404 Preview mode is read-only` for any non-GET while that cookie is present. Preview is not indexable.
**Acceptance:** a mutation attempted in preview mode is refused.
- [ ] **FH-2.7 — Immutable revisions, rollback, clone** · M · new section, `PHASE-9-…`
`version = max(version) + 1`, immutable snapshot row, `publishedRevision` pointer flipped in the same transaction. Rollback creates a new revision; history is never rewritten. Clone copies design + catalog assignments, **forces inventory to 0**, never copies domains/customers/orders/secrets, and walks the category tree topologically with explicit cycle detection.
**Acceptance:** rollback restores the chosen revision and leaves live inventory untouched.
- [ ] **FH-2.8 — Append-only inventory journal** · S · `PHASE-3-CATALOG-OFFER-FULFILLMENT-CONTRACT.md`
Every stock change writes `reason`, `referenceType`, `referenceId`, `actorId`, resulting balance. Direct answer to the v3.1 "we cannot explain your numbers" complaint.
**Acceptance:** any current quantity is reconstructible from the journal alone.
- [ ] **FH-2.9 — Per-tenant encrypted credentials** · S · `PHASE-1` / `PHASE-7`
AES-256-GCM, versioned envelope `v1.iv.tag.ciphertext` (base64url), 32-byte key from the environment. Decrypted only inside the service; never serialized into any response. Redirect/callback URLs built backend-side and allowlisted.
**Acceptance:** no credential appears in any API response, JS bundle, or browser storage.
- [ ] **FH-2.10 — Server-side storefront config validation** · M · `PHASE-10-CONTENT-MODULES-CONTRACT.md`
The server re-runs our editor's validation. Clamp-and-fallback ergonomics: clamp out-of-range numbers rather than rejecting; blank a URL that is not local `/path` or `https://` rather than erroring; fall back an invalid colour. Cap sections per page and IDs per list.
**Acceptance:** a hand-crafted API call cannot store a config the editor would have refused.
- [ ] **FH-2.11 — Digital goods** · M · `PHASE-3-…`
`FulfillmentMode: MANUAL | CODE_POOL`. `DigitalCode` pool with `AVAILABLE/RESERVED/ASSIGNED/REVOKED`, encrypted value, `valueHash` unique per `(marketplace, variant)`. Codes revealed only when the order is `PAID`/`PROCESSING`/`FULFILLED`.
**Acceptance:** an unpaid order never returns a code.
- [ ] **FH-2.12 — Marketplace status machine** · S · `PHASE-9-…`
`DRAFT → DOMAIN_PENDING → READY → ACTIVE → SUSPENDED`, with `DOMAIN_PENDING` as a real state rather than an error condition.
- [ ] **FH-2.13 — Order public token, not sequential IDs** · S · `PHASE-2-ORDERS-NOTIFICATIONS-CONTRACT.md`
Orders are addressed publicly by a random `base64url` token. Order line items carry an immutable snapshot of name, SKU, price, currency, delivery, and contact data at purchase time.
- [ ] **FH-2.14 — Order-manager as a separate contour** · M · `TRACK-S-…`
Separate URL, shell, cookie, and login; scoped to assigned marketplaces via **membership rows, not an environment variable** (their env-pinned slug is the one part not to copy). No visibility into catalog, design, domains, payment settings, or platform users. PII masked in lists, revealed in detail only with permission, and both export and reveal are logged.
- [ ] **FH-2.15 — CSV marketplace import with `dryRun` default true** · S · `PARTNER-PROVISIONING-API-CONTRACT.md`
Bulk tenant creation as a first-class operation: validate fully without writing, report create/update/skip/error per row, then confirm.
---
## Wave 3 — Proof (Lane A)
- [ ] **FH-3.1 — E2E: concurrent purchase of the last unit** · M · `e2e/`
Their §22 scenario 3. Two sessions race for the final unit; exactly one payable order results, the other gets a clean out-of-stock state.
- [ ] **FH-3.2 — E2E: replayed webhook** · M · `e2e/`
Their §22 scenario 10. The same provider event delivered twice does not complete the order twice or move stock twice.
- [ ] **FH-3.3 — Bundle budget as a blocking CI check** · S · `angular.json`, `.github/workflows/`
Currently 1.15 MB against a 700 kB budget — 452 kB over, and the only performance criticism of us that is objectively measured. Make the budget fail the build.
- [ ] **FH-3.4 — Split storefront / editor / backoffice deployables** · L · `angular.json`, routes
The single deployable bundle is both the bundle-size cause and a real separation-of-concerns problem: shipping editor and admin code to every anonymous storefront visitor. Separate bundles, separate budgets, separate CSP.
**Done when:** the storefront entry point is under budget and contains no admin or editor code.
- [ ] **FH-3.5 — Bundle secret scan in CI** · S · `.github/workflows/`
Grep the built output for credential patterns (`authorization-key`, `userid-value`, partner ID shapes, any `client_secret`). Fails the build on a hit. Cheap insurance against FH-1.3 regressing.
---
## Wave 4 — Identity: VK ID + Yandex ID (Lane C, `@marketplaces/auth`)
Blocked on **FH-0.1**. Nothing to copy from the archive — it has zero VK/Yandex/OAuth code. We take the session-issuing shape of their Telegram flow and terminate both providers into it.
- [ ] **FH-4.1 — Provider-agnostic social identity surface** · M
Collapse `VkIdGateway` into `SocialIdentityGateway`:
```ts
export type SocialProvider = 'vk' | 'yandex';
export interface SocialIdentityGateway {
getAuthorizeUrl(provider: SocialProvider, returnTo?: string): Observable<string>;
listIdentities(): Observable<ExternalIdentity[]>;
unlink(provider: SocialProvider): Observable<void>;
}
```
Touches: `src/app/core/identity/services/vk-id-gateway.interface.ts`, `vk-id-api.gateway.ts`, `vk-id-local.gateway.ts`, `vk-id-gateway.token.ts`, `src/app/components/vk-id-login/``social-login-button`. Add `'yandex_id'` to `ExternalIdentityProvider` in `core/identity/models/customer-identity.model.ts`.
- [ ] **FH-4.2 — Move PKCE ownership to the backend** · S · Lane B + C
Today `completeCallback(code, codeVerifier)` forces the browser to generate and hold the verifier. We are a confidential client. Backend generates `state` + `code_verifier`, stores them single-use for 10 minutes, handles the callback, and redirects. `completeCallback()` leaves the frontend entirely.
Contract endpoints: `GET /api/identity/v1/{provider}/authorize`, `GET /api/identity/v1/{provider}/callback`, `POST /{provider}/unlink`, `GET /me/identities`. Update `PHASE-8-IDENTITY-MESSAGING-CONTRACT.md` §2.
- [ ] **FH-4.3 — `ExternalIdentity` model** · S · Lane B
```prisma
@@unique([provider, providerUserId]) // one provider account -> one customer
```
Conflict is **not** an upsert: a `providerUserId` already bound to a different `Customer` routes to controlled resolution. The unique index makes the database refuse a silent rebind. Per-tenant OAuth app config stored encrypted (same envelope as FH-2.9): `{ clientId, clientSecret, scopes[], redirectUri }`.
- [ ] **FH-4.4 — VK ID** · M
OAuth 2.1, PKCE mandatory (S256). Authorize `https://id.vk.com/authorize`; token `POST https://id.vk.com/oauth2/auth`; profile `POST https://id.vk.com/oauth2/user_info`; logout `https://id.vk.com/oauth2/logout` on unlink.
**Trap to write into the contract:** the callback returns `device_id` alongside `code`, and the token exchange fails without it. This is the most common VK ID integration bug.
VK often does not return an email — email must stay optional on `Customer`.
- [ ] **FH-4.5 — Yandex ID** · S · *after FH-4.4*
OAuth 2.0 with PKCE. Authorize `https://oauth.yandex.ru/authorize`; token `POST https://oauth.yandex.ru/token` with HTTP Basic `client_id:client_secret`; profile `GET https://login.yandex.ru/info?format=json` with header `Authorization: OAuth <token>``id`, `login`, `default_email`, `default_phone`, `psuid`.
A second strategy object against the same surface — roughly a day once VK works.
*Confirm exact parameter and scope names against live provider docs; both providers revised their flows recently.*
- [ ] **FH-4.6 — Migrate Telegram onto `ExternalIdentity`** · M · Lane B + C
Telegram becomes one provider among several rather than the schema's only key. Ends the shared customer/admin Telegram session their audit flagged.
- [ ] **FH-4.7 — Account linking UI** · M
`/me/identities` — show linked providers, link, unlink, and surface the conflict-resolution path from FH-4.3.
- [ ] **FH-4.8 — Email/phone OTP repositioned as recovery** · S · Lane E
Our approved [email/phone login spec](superpowers/specs/2026-08-15-email-phone-login-design.md) stays valid but drops below VK ID and becomes the fallback when a messenger channel is unavailable, per v3.1 §14.
---
## Continuous — Ops (Lane D)
- [ ] **FH-D.1 — Proven restore drill** · M
We have deploy automation and no proven restore. Add a restore-check script and schedule it. Their `restore-check.sh` + WAL archiving (`wal_level=replica`, `archive_mode=on`, `archive_timeout=300`) is the model.
**Done when:** a restore into a clean environment has been executed and its result recorded.
- [ ] **FH-D.2 — Database unreachable from the internet, structurally** · S · Lane B/D
Data network `internal: true`; API bound to loopback only; `no-new-privileges` on every service. Makes it a property of the topology rather than a firewall promise.
- [ ] **FH-D.3 — Host hardening we lack** · M
fail2ban jail, sshd hardening drop-in, sysctl hardening, scoped sudoers per deploy role. Add to `scripts/deploy/server-setup.sh`.
*Keep ours where ours is better:* `add-domain.sh` already pre-checks the DNS A record and runs `nginx -t` before and after; `server-setup.sh` already configures ufw. Do **not** copy their hardcoded server IP.
---
## Continuous — Process (Lane E)
- [ ] **FH-E.1 — Adopt the nine invariants as a signed acceptance gate** · S
From their handoff §7: tenant by Host not by a browser-supplied ID; order price computed backend; stock and reservation atomic; payment webhook idempotent; credentials never leave the backend; published revision immutable; design rollback does not roll back live inventory; no user reads an unassigned marketplace through UI or API; every admin mutation leaves an audit trail.
Put them at the head of `docs/backend/BACKEND-HANDOFF.md` as gates, not aspirations.
- [ ] **FH-E.2 — PR policy** · S
One functional area per PR. Mandatory: purpose, screenshots, API changes, migrations, test evidence, security impact, rollback plan. Never change payment/inventory/order state machines inside a redesign PR.
- [ ] **FH-E.3 — Release discipline** · S
"A local build or the existence of a UI does not mean production readiness." Every release records version, migrations, healthcheck, smoke result, dependency audit, and a rollback path.
- [ ] **FH-E.4 — ADR for the harvest** · S
Record the decision: adopt these improvements, reject their architecture, keep our frontend and governance. Include the §9 rejection list so it does not get relitigated.
- [ ] **FH-E.5 — Reduce `localStorage` to cache, never truth** · M · Lane A
19 files touch `localStorage`, mostly admin facades and `project-editor-draft-storage.service.ts`. Their disqualifying objection is not "you use localStorage" — it is "localStorage is your source of truth."
**Do:** keep local drafts as an offline convenience with an explicit "unsaved local draft" indicator and server-wins reconciliation; never let a local value be the published state.
**Done when:** no admin or editor write path can publish without a server round-trip.
---
## Scoreboard
| Wave | Items | Lane | Blocked by |
|---|---:|---|---|
| 0 — Decide | 2 | C, A | — |
| 1 — Live defects | 4 | A | FH-0.2 (one item) |
| 2 — Contracts | 15 | B | — |
| 3 — Proof | 5 | A | — |
| 4 — Identity | 8 | C | FH-0.1 |
| Ops | 3 | D | — |
| Process | 5 | E | — |
| **Total** | **42** | | |
**Start here:** FH-0.1 (escalate today, it is a one-way door), then FH-1.1 and FH-1.4 — both are small, both are live defects, and both close findings their audit will otherwise keep raising.