Some checks failed
Architecture Governance / architecture (push) Has been cancelled
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
237 lines
21 KiB
Markdown
237 lines
21 KiB
Markdown
# 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)
|
|
|
|
- [x] **FH-1.1 — Kill the plaintext third-party geo call** · S · Lane A · **done 2026-08-21**
|
|
Now `GET {tenantApiBase}/geo/resolve`, same base as `/regions`. Server reads the client IP; nothing leaves our infrastructure. Endpoint specified in [BACKEND-API-REFERENCE.md](../BACKEND-API-REFERENCE.md) §6 — **not built yet**, and until it is the client falls back to the manual picker, which is what production has effectively had all along. Covered by `src/app/services/location.service.spec.ts` (4 tests, one of which fails the build on any off-origin or plaintext request from this service).
|
|
*Was:* `location.service.ts:75` called `http://ip-api.com/json/?fields=…` from an HTTPS origin. Mixed active content is blocked, so `detectLocation()` only ever took its error branch — auto-detect was dead in production, not merely insecure — and the attempt still leaked every visitor's IP to a third party.
|
|
|
|
- [ ] **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.
|
|
|
|
- [x] **FH-1.3 — Remove provider credentials from the browser** · M · Lane A · **landed via the `@marketplaces/payment` migration**
|
|
The legacy payment surface on `ApiService` was deleted wholesale in that work. `grep -ri "authorization-key\|userid-value\|web-97ec" src/` now returns nothing. Keep FH-3.5 (bundle secret scan) to stop it coming back.
|
|
*Was:* `api.service.ts:675` set `authorization-key` and `userid-value` headers client-side, and `api.service.ts:143` shipped a partner ID literal in the bundle. Their audit's most serious finding, and it was correct.
|
|
|
|
- [ ] **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.
|
|
|
|
- [x] **FH-3.3 — Bundle budget as a blocking CI check** · S · **done 2026-08-21**
|
|
`maximumError` on the initial bundle lowered `1.8MB → 1.6MB` in `angular.json`. Measured today: **1.55 MB raw / 324.58 kB transfer** — worse than the 1.15 MB they measured on 11 Aug, so this had been growing unwatched. The threshold is a **ratchet**, not the target: set just above today's size so the bundle cannot grow, with the 700 kB warning left in place as the goal. Lower it every time the number comes down. CI now runs the production build (`npm run build` already defaults to production).
|
|
|
|
- [ ] **FH-3.4 — Get the initial bundle down** · L · `angular.json`, `src/app/`
|
|
**Premise corrected after measuring.** Admin, editor, catalog, cart and the `en`/`hy` locales are *already* lazy chunks — nothing admin-shaped ships to an anonymous visitor. The entire 1.55 MB is `main` alone. So this is not a "split the deployables" job; it is a "find what is eager" job.
|
|
Known contributors: `src/app/i18n/ru.ts` (141 kB of source, default locale, eager while `en`/`hy` are lazy), the eagerly-provided core services and their DI tokens, `icon-registry.ts`.
|
|
Also: `qrcode`, pulled in by `@marketplaces/auth`, is not ESM and causes an optimizer bailout — worth fixing in the package.
|
|
**Done when:** initial is under the 700 kB warning, with the ratchet lowered in steps along the way.
|
|
|
|
- [x] **FH-3.5 — Bundle secret scan in CI** · S · **done 2026-08-21**
|
|
`scripts/ci/scan-bundle.sh`, wired as `npm run scan:bundle` and a CI step after Build. Seven patterns: both provider auth headers, the partner ID shape, `client_secret`, private key blocks, AWS keys, Telegram bot tokens. Verified in both directions — clean against the real `dist/`, and fails with exit 1 against a planted credential.
|
|
|
|
---
|
|
|
|
## 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.6 — Keep mock gateways out of production builds** · M · Lane A
|
|
Measured 2026-08-21: mock seed data reaches the production bundle. `ptr_local`, a fixture literal from `partner-hierarchy-local.gateway.ts`, is present in a built lazy chunk. Cause: 21 DI tokens use `factory: () => (environment.useMockData ? inject(XLocalGateway) : inject(XApiGateway))`, and referencing both branches keeps both classes reachable, so the optimizer cannot drop the mock. 75 kB of local-gateway source, plus its fixtures, ships to users.
|
|
This is the concrete form of their strongest objection — "mock repositories as production implementation" — and it is mechanical to fix. The pattern to copy is already in this repo: `mock-data.interceptor.production.ts` swapped in via `fileReplacements`.
|
|
**Done when:** `scan-bundle.sh` can gate on mock fixture markers and pass.
|
|
|
|
- [ ] **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 | 6 | E | — |
|
|
| **Total** | **43** | | |
|
|
|
|
**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.
|