docs: add 2026-06-10 audit and remediation planning documents
- Comprehensive Workspace Audit - 2026-06-10.md - C1-Secrets-Rotation-Checklist-2026-06-10.md - Mistral-Outsource-Package-2026-06-10.md - Workflow-Remediation-Plan-2026-06-10.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
76
09 - Audits/C1-Secrets-Rotation-Checklist-2026-06-10.md
Normal file
76
09 - Audits/C1-Secrets-Rotation-Checklist-2026-06-10.md
Normal file
@@ -0,0 +1,76 @@
|
||||
---
|
||||
title: C1 Secrets Rotation Checklist - 2026-06-10
|
||||
tags: [audit, security, secrets, rotation, c1]
|
||||
created: 2026-06-10
|
||||
status: in-progress
|
||||
---
|
||||
|
||||
# C1 Secrets Rotation Checklist - 2026-06-10
|
||||
|
||||
## 1. Tracked env files
|
||||
|
||||
deployment/.env and deployment/.env.dev are tracked in git.
|
||||
|
||||
- [ ] Rotate ALL credential values via provider dashboards first
|
||||
- [ ] Create deployment/.env.example and deployment/.env.dev.example with placeholders
|
||||
- [ ] Add deployment/.env and deployment/.env.dev to .gitignore
|
||||
- [ ] Run: git rm --cached deployment/.env deployment/.env.dev
|
||||
- [ ] Commit the removal
|
||||
- [ ] History cleanup only after rotation confirmed
|
||||
|
||||
## 2. Test and source files with key-shaped material — triage each
|
||||
|
||||
For each, triage as real vs fake test fixture:
|
||||
|
||||
- backend/__tests__/decentralized-payment-verifier.test.ts
|
||||
- backend/__tests__/payment-edge-cases.test.ts
|
||||
- backend/__tests__/payment-integration.test.ts
|
||||
- backend/__tests__/request-network-webhook.test.ts
|
||||
- backend/__tests__/sweep-service.test.ts
|
||||
- backend/__tests__/transaction-safety-provider.test.ts
|
||||
- backend/src/services/payment/decentralizedPaymentService.ts
|
||||
- backend/usdt-reset-test-report.md
|
||||
- scanner/balance_test.go
|
||||
- scanner/config.go
|
||||
- nick-doc/01 - Architecture/Request Network Integration Constraints.md
|
||||
- nick-doc/08 - Operations/Handoff - RN Multichain Probe - 2026-05-28.md
|
||||
- nick-doc/10 - Services/scanner.md
|
||||
- nick-doc/11 - Testing/Escrow Marketplace E2E Procedure.md
|
||||
|
||||
For real keys: rotate → replace with process.env.VAR_NAME → add to .env.example
|
||||
|
||||
For test fixtures: replace with obviously-fake value, add // test fixture comment
|
||||
|
||||
- [ ] backend/__tests__/decentralized-payment-verifier.test.ts
|
||||
- [ ] backend/__tests__/payment-edge-cases.test.ts
|
||||
- [ ] backend/__tests__/payment-integration.test.ts
|
||||
- [ ] backend/__tests__/request-network-webhook.test.ts
|
||||
- [ ] backend/__tests__/sweep-service.test.ts
|
||||
- [ ] backend/__tests__/transaction-safety-provider.test.ts
|
||||
- [ ] backend/src/services/payment/decentralizedPaymentService.ts
|
||||
- [ ] backend/usdt-reset-test-report.md
|
||||
- [ ] scanner/balance_test.go
|
||||
- [ ] scanner/config.go
|
||||
- [ ] nick-doc/01 - Architecture/Request Network Integration Constraints.md
|
||||
- [ ] nick-doc/08 - Operations/Handoff - RN Multichain Probe - 2026-05-28.md
|
||||
- [ ] nick-doc/10 - Services/scanner.md
|
||||
- [ ] nick-doc/11 - Testing/Escrow Marketplace E2E Procedure.md
|
||||
|
||||
## 3. Documentation files
|
||||
|
||||
- [ ] Replace any key values in nick-doc/ with [REDACTED] or truncated form (0xfcE8...CdbA)
|
||||
|
||||
## 4. Git history cleanup (ONLY after rotation confirmed)
|
||||
|
||||
- [ ] All rotated credentials live and all code instances replaced
|
||||
- [ ] Notify ALL contributors — history rewrite requires re-cloning
|
||||
- [ ] Use git filter-repo or BFG Repo Cleaner
|
||||
- [ ] Force-push all affected branches (requires explicit user approval)
|
||||
|
||||
## 5. Prevention
|
||||
|
||||
- [ ] Verify .gitignore blocks .env variants
|
||||
- [ ] Confirm deployment/.gitleaks.toml is active
|
||||
- [ ] Add gitleaks pre-commit hook: gitleaks protect --staged --config deployment/.gitleaks.toml
|
||||
- [ ] Add gitleaks scan to Woodpecker CI pipeline
|
||||
- [ ] Add to AGENTS.md: test keys must use process.env references, never inline values
|
||||
457
09 - Audits/Comprehensive Workspace Audit - 2026-06-10.md
Normal file
457
09 - Audits/Comprehensive Workspace Audit - 2026-06-10.md
Normal file
@@ -0,0 +1,457 @@
|
||||
---
|
||||
title: Comprehensive Workspace Audit - 2026-06-10
|
||||
tags: [audit, security, frontend, backend, deployment, scanner, dependencies, multi-shop]
|
||||
created: 2026-06-10
|
||||
updated: 2026-06-10
|
||||
status: open
|
||||
---
|
||||
|
||||
# Comprehensive Workspace Audit - 2026-06-10
|
||||
|
||||
Full workspace audit across nested Git repositories under `/Users/manwe/CascadeProjects/escrow`.
|
||||
|
||||
Primary product focus was the multi-shop branch:
|
||||
|
||||
- `frontend/`: `feature/white-label-shops`
|
||||
- `backend/`: `feature/white-label-shops`
|
||||
|
||||
No code, build, deployment, pipeline, Docker, or secret files were changed during this audit.
|
||||
|
||||
## Scope
|
||||
|
||||
| Repo | Branch audited | Status at audit time | Notes |
|
||||
|---|---|---|---|
|
||||
| `frontend/` | `feature/white-label-shops` | Dirty worktree, ahead of remote | Multi-shop UI, tenant admin UI, Telegram Mini App, wallet/payment flows. |
|
||||
| `backend/` | `feature/white-label-shops` | Clean worktree, ahead of remote | Tenant routes, storefront routes, payment services, file services, webhooks, scanner integration. |
|
||||
| `deployment/` | `main` | Dirty worktree, ahead of remote | `escrow-multi` stack and environment material. |
|
||||
| `scanner/` | `development` | Clean worktree, ahead of remote | Go payment scanner and balance-watch service. |
|
||||
| `amanat-assist/` | `main` | Dirty worktree | Assist frontend plus local LLM proxy. |
|
||||
| `nick-doc/` | `main` | Dirty worktree | Documentation vault, tenant docs, prior audits. |
|
||||
|
||||
Related lighter repo/documentation scan: [[Multi-Shop Branch Project Scan - 2026-06-10]].
|
||||
|
||||
## Method
|
||||
|
||||
- Read project instructions from root `AGENTS.md`, root `RTK.md`, and `nick-doc/AGENTS.md`.
|
||||
- Enumerated all nested Git repositories.
|
||||
- Confirmed frontend/backend were on `feature/white-label-shops`.
|
||||
- Reviewed mounted backend routes and service boundaries for auth, tenant isolation, file access, payment state, webhooks, and scanner integration.
|
||||
- Reviewed frontend app routes, auth/token storage, debug surfaces, API proxying, and dependency/runtime quality.
|
||||
- Reviewed deployment compose files and tracked environment-file posture without printing secret values.
|
||||
- Ran a sanitized secret scan that reported only file/path/line/pattern class, never values.
|
||||
- Ran available read-only verification commands.
|
||||
|
||||
## Executive Summary
|
||||
|
||||
The most urgent risks are not cosmetic. They are live operational/security risks:
|
||||
|
||||
1. Tracked deployment `.env` files and additional key-shaped literals need immediate secret rotation and history cleanup.
|
||||
2. The frontend-to-assist LLM path is unauthenticated and can proxy arbitrary model calls.
|
||||
3. Tenant bot claim URLs are returned from a broad bot-list route and can leak capability tokens to lower tenant roles.
|
||||
4. Generic file delete/info routes authorize only "logged in", not file ownership.
|
||||
5. The Request Network intent route still defaults to trusting client-supplied payment amount unless oracle quoting is explicitly enabled.
|
||||
6. Several payment routes compare JWT user ids directly against Postgres UUID payment fields, causing false-deny and inconsistent checkout/payment behavior.
|
||||
|
||||
## Finding Register
|
||||
|
||||
| ID | Severity | Area | Status |
|
||||
|---|---|---|---|
|
||||
| C1 | Critical | Secrets and credentials | Open |
|
||||
| C2 | Critical | LLM proxy exposure | Open |
|
||||
| H1 | High | Tenant bot claim authorization | Open |
|
||||
| H2 | High | File delete/info authorization | Open |
|
||||
| H3 | High | Client-trusted payment amount | Open |
|
||||
| H4 | High | Payment UUID/JWT identity mismatch | Open |
|
||||
| H5 | High | Dependency advisories | Open |
|
||||
| M1 | Medium | Frontend typecheck bypass in builds | Open |
|
||||
| M2 | Medium | Browser token storage | Open |
|
||||
| M3 | Medium | Permit relay ownership/rate limit | Open |
|
||||
| M4 | Medium | Production debug surface | Open |
|
||||
| M5 | Medium | Scanner operational auth footgun | Open |
|
||||
| M6 | Medium | Backend/frontend lint health | Open |
|
||||
| L1 | Low | Deployment/dev defaults | Open |
|
||||
| L2 | Low | File upload reliability and MIME hardening | Open |
|
||||
|
||||
## Critical Findings
|
||||
|
||||
### C1 - Tracked env files and key-shaped material require rotation
|
||||
|
||||
**Evidence**
|
||||
|
||||
- `deployment/.env` is tracked.
|
||||
- `deployment/.env.dev` is tracked.
|
||||
- `deployment/escrow-multi/docker-compose.yml` loads `.env` directly.
|
||||
- Sanitized scan found token-shaped assignments in tracked deployment env files.
|
||||
- Sanitized scan found private-key-shaped or key-like hex material in backend tests/reports/source, scanner tests/config/comments, and docs.
|
||||
|
||||
Representative locations, values intentionally omitted:
|
||||
|
||||
- `deployment/.env`
|
||||
- `deployment/.env.dev`
|
||||
- `backend/__tests__/decentralized-payment-verifier.test.ts`
|
||||
- `backend/__tests__/payment-edge-cases.test.ts`
|
||||
- `backend/__tests__/payment-integration.test.ts`
|
||||
- `backend/__tests__/request-network-webhook.test.ts`
|
||||
- `backend/__tests__/sweep-service.test.ts`
|
||||
- `backend/__tests__/transaction-safety-provider.test.ts`
|
||||
- `backend/src/services/payment/decentralizedPaymentService.ts`
|
||||
- `backend/usdt-reset-test-report.md`
|
||||
- `scanner/balance_test.go`
|
||||
- `scanner/config.go`
|
||||
- `nick-doc/01 - Architecture/Request Network Integration Constraints.md`
|
||||
- `nick-doc/08 - Operations/Handoff - RN Multichain Probe - 2026-05-28.md`
|
||||
- `nick-doc/10 - Services/scanner.md`
|
||||
- `nick-doc/11 - Testing/Escrow Marketplace E2E Procedure.md`
|
||||
|
||||
**Impact**
|
||||
|
||||
Tracked env files and any real private-key material must be treated as exposed. If these values were ever valid, repository history, backups, forks, and local clones can retain them.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Rotate all credentials found in tracked env files.
|
||||
- Triage key-shaped literals into fake/test fixture vs real. Rotate any value that was ever used.
|
||||
- Replace test keys with generated fixtures or env-var references.
|
||||
- Move env files to untracked local templates: `.env.example`, `.env.dev.example`.
|
||||
- Add ignore rules and pre-commit/CI secret scanning.
|
||||
- History-clean only after rotation plan is agreed, because rewrite affects every clone.
|
||||
|
||||
### C2 - Public unauthenticated LLM proxy path
|
||||
|
||||
**Evidence**
|
||||
|
||||
- `frontend/src/app/api/llm/route.ts:8` accepts POST requests with no auth, rate limit, schema check, or body-size cap, then forwards arbitrary JSON to `LLM_PROXY_URL`.
|
||||
- `amanat-assist/llm-proxy/index.mjs:73` defaults CORS to all origins when `ALLOWED_ORIGINS` is empty.
|
||||
- `amanat-assist/llm-proxy/index.mjs:96` reads the full request body without a hard cap.
|
||||
- `amanat-assist/llm-proxy/index.mjs:128` accepts caller-chosen `provider` and `model`.
|
||||
- `amanat-assist/llm-proxy/index.mjs:180` logs upstream error data.
|
||||
|
||||
**Impact**
|
||||
|
||||
Any unauthenticated internet client that can reach the frontend route can spend provider quota, probe internal proxy behavior, and send unbounded payloads. If prompts include sensitive user data, the route also becomes an ungoverned data egress path.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Require authenticated Amanat session or service-to-service token on `/api/llm`.
|
||||
- Add per-user and per-IP rate limits.
|
||||
- Validate request schema and allowlist provider/model.
|
||||
- Enforce body-size caps at the Next.js route and proxy.
|
||||
- Restrict CORS to known origins.
|
||||
- Redact/log only status, provider, model class, and request id.
|
||||
|
||||
## High Findings
|
||||
|
||||
### H1 - Tenant bot claim URL leaks through broad bot listing
|
||||
|
||||
**Evidence**
|
||||
|
||||
- `backend/src/services/tenant/tenantBotService.ts:75-89` returns `claimUrl` for pending bots.
|
||||
- `backend/src/services/tenant/tenantBotService.ts:268-270` maps all tenant bot rows through that public serializer.
|
||||
- `backend/src/routes/tenantRoutes.ts:510-518` allows `owner`, `manager`, `finance`, `support`, and `developer` to list bots.
|
||||
|
||||
**Impact**
|
||||
|
||||
The claim URL contains a capability token. A lower-privileged tenant role that can list bots can obtain a pending bot claim link and potentially claim Telegram admin control for the bot.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Remove `claimUrl` from the generic bot-list response.
|
||||
- Keep claim URLs behind the existing owner/developer claim-link route or create a dedicated high-privilege capability endpoint.
|
||||
- Store only a hashed claim token if practical.
|
||||
- Add tests for support/finance/manager not receiving claim material.
|
||||
|
||||
### H2 - Generic file delete/info routes do not enforce ownership
|
||||
|
||||
**Evidence**
|
||||
|
||||
- `backend/src/services/file/fileRoutes.ts:71-82` exposes delete routes to any authenticated user.
|
||||
- `backend/src/services/file/fileRoutes.ts:86-88` exposes file-info route to any authenticated user.
|
||||
- `backend/src/services/file/fileController.ts:247-275` checks only that a user is authenticated before deleting.
|
||||
- `backend/src/services/file/fileController.ts:278-299` checks only that a user is authenticated before returning file info.
|
||||
- `backend/src/services/file/fileService.ts:30-50` safely confines paths to upload root, so this is an authorization issue rather than arbitrary filesystem traversal.
|
||||
|
||||
**Impact**
|
||||
|
||||
Any logged-in user can target public-upload files under the upload root if they know or guess the path. That can delete avatars, product/request-template/blog assets, or query metadata for files they do not own.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Replace path-based mutation with file ids tied to an owner/resource.
|
||||
- Require owner, admin, or resource participant checks before delete/info.
|
||||
- Keep the existing upload-root confinement.
|
||||
- Add tests for cross-user delete/info denial.
|
||||
|
||||
### H3 - Payment intent route defaults to client-trusted amount
|
||||
|
||||
**Evidence**
|
||||
|
||||
- `backend/src/services/payment/requestNetwork/requestNetworkRoutes.ts:41-42` enables oracle quoting only when `ORACLE_QUOTING_ENABLED` is exactly `true`.
|
||||
- `backend/src/services/payment/requestNetwork/requestNetworkRoutes.ts:599-678` computes amount server-side only when the flag is enabled.
|
||||
- `backend/src/services/payment/requestNetwork/requestNetworkRoutes.ts:680-689` legacy path trusts client `amount`.
|
||||
|
||||
**Impact**
|
||||
|
||||
If the flag is absent or false in production, a buyer can submit a lower amount than the seller offer requires. The code comment itself marks this as a risk and says to remove the branch after cut-over.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Make server-side quoting the default and remove the client-trusted fallback.
|
||||
- Fail closed if seller offer/profile cannot be loaded.
|
||||
- Persist quote inputs and outputs for auditability.
|
||||
- Add regression tests that client amount is ignored or rejected.
|
||||
|
||||
### H4 - Payment route authorization mixes legacy ids and Postgres UUIDs
|
||||
|
||||
**Evidence**
|
||||
|
||||
- `backend/src/db/repositories/drizzle/DrizzlePaymentRepo.ts:915-932` resolves and stores `buyerId` as a Postgres UUID.
|
||||
- `backend/src/services/payment/requestNetwork/requestNetworkRoutes.ts:390-392` compares `payment.buyerId` directly to JWT user id.
|
||||
- `backend/src/services/payment/paymentRoutes.ts:417-418`, `440-442`, and `464-467` compare direct payment ids to JWT user id.
|
||||
|
||||
**Impact**
|
||||
|
||||
Legitimate buyers can be denied checkout reload, status, or confirmation when JWT ids are legacy ObjectIds but payment rows store UUIDs. Inconsistent route behavior also makes payment support and debugging brittle.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Reuse the canonical access helper already present in `paymentController.ts`.
|
||||
- Normalize user/payment participant checks through one identity utility.
|
||||
- Add regression tests for legacy JWT id against UUID-backed payment rows.
|
||||
|
||||
### H5 - Production dependency advisories remain open
|
||||
|
||||
**Evidence**
|
||||
|
||||
Frontend production dependency audit:
|
||||
|
||||
- 2 critical
|
||||
- 16 high
|
||||
- 39 moderate
|
||||
- 8 low
|
||||
- Largest critical/high cluster: `protobufjs` via Trezor dependencies in `frontend/yarn.lock`.
|
||||
|
||||
Backend production dependency audit:
|
||||
|
||||
- 7 high
|
||||
- 7 moderate
|
||||
- Notable packages: `axios`, `jws`, `lodash`, `path-to-regexp`, `socket.io-parser`, `validator`, `ws`.
|
||||
|
||||
Amanat Assist production dependency audit:
|
||||
|
||||
- 0 vulnerabilities reported.
|
||||
|
||||
**Impact**
|
||||
|
||||
Wallet, websocket, HTTP, validation, and JWT-adjacent packages are part of high-risk surfaces. Some advisories are transitive and may require careful upgrade testing, but they should not stay invisible in release planning.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Update lockfiles in a controlled dependency-hardening branch.
|
||||
- Prioritize frontend `protobufjs`/Trezor path and backend `axios`, `jws`, `socket.io-parser`, `validator`, `path-to-regexp`.
|
||||
- Run payment, wallet, Telegram, socket, and checkout smoke tests after upgrades.
|
||||
|
||||
## Medium Findings
|
||||
|
||||
### M1 - Frontend builds skip TypeScript errors
|
||||
|
||||
**Evidence**
|
||||
|
||||
- `frontend/next.config.ts:27-29` sets `typescript: { ignoreBuildErrors: true }`.
|
||||
- Frontend lint also reports `@ts-nocheck` in payment components.
|
||||
|
||||
**Impact**
|
||||
|
||||
Production builds can ship TypeScript failures. This is especially risky while multi-shop, Telegram Mini App, and payment code are changing quickly.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Remove `ignoreBuildErrors` once current type issues are cleaned.
|
||||
- Add a separate `tsc --noEmit` CI gate if Next build must stay fast.
|
||||
|
||||
### M2 - Browser token storage increases XSS blast radius
|
||||
|
||||
**Evidence**
|
||||
|
||||
- `frontend/src/auth/context/jwt/auth-provider.tsx:35` reads `accessToken` from localStorage.
|
||||
- `frontend/src/lib/axios.ts:69-72` sends localStorage token as bearer auth.
|
||||
- `frontend/src/lib/axios.ts:127-141` still supports legacy localStorage refresh token cleanup.
|
||||
- `amanat-assist/src/services/auth.ts:38-57` persists access/refresh token state to localStorage.
|
||||
- `amanat-assist/src/services/auth.ts:152-163` accepts OAuth tokens from URL query params.
|
||||
|
||||
**Impact**
|
||||
|
||||
Any XSS can extract bearer tokens. Query-param token handoff can also leak through browser history, analytics, referrers, or logs before cleanup.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Move toward httpOnly session cookies or a backend-for-frontend pattern for browser sessions.
|
||||
- Stop carrying access/refresh tokens in URL query params.
|
||||
- Add strict CSP and minimize inline script risk.
|
||||
- Keep localStorage only for non-sensitive UI state.
|
||||
|
||||
### M3 - Permit relay route lacks ownership check and rate limit
|
||||
|
||||
**Evidence**
|
||||
|
||||
- `backend/src/services/payment/requestNetwork/requestNetworkRoutes.ts:257-324` relays a signed permit for a payment after validating rail/spender/value.
|
||||
- The route is authenticated but does not verify the requester owns the payment.
|
||||
- The code comment at `requestNetworkRoutes.ts:255-256` notes it should be rate-limited per buyer.
|
||||
|
||||
**Impact**
|
||||
|
||||
This can leak pending payment existence and can cause relayer gas spend attempts for payments the requester does not own, provided they have a valid permit payload.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Require buyer/admin access before permit validation and relay.
|
||||
- Add per-buyer/per-payment rate limiting.
|
||||
- Return indistinguishable 404/403 behavior if needed to reduce enumeration.
|
||||
|
||||
### M4 - Telegram debug panel can show production operational/user data
|
||||
|
||||
**Evidence**
|
||||
|
||||
- `frontend/src/components/debug/telegram-debug-panel.tsx:44-56` shows the panel in production for Mini App context or explicit debug request.
|
||||
- `frontend/src/components/debug/telegram-debug-panel.tsx:73-96` displays API/socket URLs, email, role, Telegram platform/version, and initData presence/length.
|
||||
|
||||
**Impact**
|
||||
|
||||
This is not direct token leakage, but it exposes user and operational diagnostics in production UI.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Gate production debug panels behind admin/developer role and signed debug mode.
|
||||
- Hide email and internal URLs unless explicitly needed.
|
||||
- Consider stripping the panel from production builds.
|
||||
|
||||
### M5 - Scanner endpoints are unauthenticated if `SCANNER_API_KEY` is missing
|
||||
|
||||
**Evidence**
|
||||
|
||||
- `scanner/config.go:115-127` reads `SCANNER_API_KEY` and logs that endpoints are unauthenticated when missing.
|
||||
- `scanner/api.go:111-126` allows all requests when the key is empty.
|
||||
|
||||
**Positive controls observed**
|
||||
|
||||
- `scanner/api.go:135-136` caps create-intent body size.
|
||||
- `scanner/security.go:58-99` validates callback URLs against SSRF rules.
|
||||
- `scanner/security.go:102-127` adds dial-time protection for public callback mode.
|
||||
|
||||
**Impact**
|
||||
|
||||
The scanner is safe only if production always sets `SCANNER_API_KEY` and ingress does not expose it unintentionally.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Fail startup in production when `SCANNER_API_KEY` is missing.
|
||||
- Keep the current dev-mode behavior only for explicit local mode.
|
||||
- Add deployment checks for scanner auth.
|
||||
|
||||
### M6 - Lint health is currently failing in frontend and backend
|
||||
|
||||
**Evidence**
|
||||
|
||||
Backend `npm run lint`:
|
||||
|
||||
- 29 errors
|
||||
- 996 warnings
|
||||
- Error classes include forbidden `require()` imports, empty blocks, and namespace usage.
|
||||
|
||||
Frontend `npx yarn@1.22.22 lint`:
|
||||
|
||||
- 83 errors
|
||||
- 65 warnings
|
||||
- Notable correctness errors include conditional React hooks in `frontend/src/sections/telegram/view/telegram-points-view.tsx:59-61` and `@ts-nocheck` in payment components.
|
||||
|
||||
**Impact**
|
||||
|
||||
Lint is not just style here. Hook ordering and `@ts-nocheck` can hide runtime failures in Telegram/payment flows.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Fix hook-rule and `@ts-nocheck` violations first.
|
||||
- Then decide whether import-sort failures should block release or be auto-fixed.
|
||||
- Keep lint gating focused enough that teams do not normalize red builds.
|
||||
|
||||
## Low Findings
|
||||
|
||||
### L1 - Deployment/dev defaults are footguns
|
||||
|
||||
**Evidence**
|
||||
|
||||
- `deployment/docker-compose.yml` and `deployment/dev-amn/docker-compose.yml` include hardcoded dev/default database or Redis passwords.
|
||||
- `deployment/escrow-multi/migrate/migrations/0018_db_privilege_isolation.sql` contains role password literal `undefined`.
|
||||
|
||||
**Impact**
|
||||
|
||||
These are not necessarily live production secrets, but defaults can become real accidentally when copied between stacks.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Replace hardcoded dev credentials with env references and clear examples.
|
||||
- Fix or remove copied migration files that embed `undefined` password literals.
|
||||
|
||||
### L2 - Upload reliability and MIME hardening gaps
|
||||
|
||||
**Evidence**
|
||||
|
||||
- General upload flows rely primarily on MIME validation.
|
||||
- Chat attachment handling has stronger magic-byte validation, but generic uploads are less strict.
|
||||
- Non-image multi-file upload code constructs output paths for documents but needs verification that files are moved/copied as expected.
|
||||
|
||||
**Impact**
|
||||
|
||||
Potential broken uploads for documents and weaker file-type assurance outside chat.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
- Reuse chat attachment magic-byte validation for all user-controlled file uploads where practical.
|
||||
- Add focused tests for multi-file document upload persistence.
|
||||
|
||||
## Positive Observations
|
||||
|
||||
- Backend mounts raw webhook body parsing before global JSON parsing for Request Network, AMN scanner, and Telegram tenant webhooks.
|
||||
- Socket.IO connection auth rejects refresh tokens and verifies room membership for chat/request scoped rooms.
|
||||
- Tenant resolution uses host/slug context rather than trusting arbitrary caller headers.
|
||||
- Scanner has meaningful SSRF defenses for callback URLs and dial-time checks in the public-callback mode.
|
||||
- Markdown rendering uses `rehypeRaw` followed by sanitization and protocol restrictions.
|
||||
- Backend `tsc --noEmit -p tsconfig.json` passed.
|
||||
- Scanner `go test ./...` passed.
|
||||
- Amanat Assist `npm run build` passed.
|
||||
- Amanat Assist `npm audit --omit=dev` reported no production vulnerabilities.
|
||||
|
||||
## Verification Commands
|
||||
|
||||
| Repo | Command | Result |
|
||||
|---|---|---|
|
||||
| `backend/` | `npm run typecheck` | Passed |
|
||||
| `backend/` | `npm run lint` | Failed: 29 errors, 996 warnings |
|
||||
| `backend/` | `npm audit --omit=dev --json` | Failed advisories: 7 high, 7 moderate |
|
||||
| `frontend/` | `npx -y yarn@1.22.22 lint` | Failed: 83 errors, 65 warnings |
|
||||
| `frontend/` | `npx -y yarn@1.22.22 audit --groups dependencies --json` | Failed advisories: 2 critical, 16 high, 39 moderate, 8 low |
|
||||
| `scanner/` | `go test ./...` | Passed |
|
||||
| `amanat-assist/` | `npm run build` | Passed |
|
||||
| `amanat-assist/` | `npm audit --omit=dev --json` | Passed: 0 vulnerabilities |
|
||||
|
||||
Frontend repo declares Yarn 1 and this shell did not have a global `yarn` binary, so frontend commands were run through `npx -y yarn@1.22.22`.
|
||||
|
||||
## Recommended Remediation Order
|
||||
|
||||
1. Rotate and scrub tracked secret material.
|
||||
2. Lock down `/api/llm` and `amanat-assist/llm-proxy`.
|
||||
3. Remove claim URLs from broad tenant bot listing.
|
||||
4. Add file ownership/resource checks to delete/info routes.
|
||||
5. Force server-side payment pricing and remove client-trusted amount fallback.
|
||||
6. Normalize payment participant authorization across UUID and legacy id paths.
|
||||
7. Upgrade vulnerable dependency clusters.
|
||||
8. Fix frontend hook-rule and `@ts-nocheck` lint failures.
|
||||
9. Re-enable strict frontend type/build checks.
|
||||
10. Harden scanner production startup around `SCANNER_API_KEY`.
|
||||
|
||||
## Notes and Guardrails
|
||||
|
||||
- Do not print, paste, or document actual secret values while remediating C1.
|
||||
- Do not change Woodpecker pipelines, Dockerfiles, deploy commands, cache/prune behavior, or production build procedure without explicit approval.
|
||||
- Frontend/backend code changes require coordinated patch version bumps before build/deploy. This documentation-only audit does not require a version bump.
|
||||
- Treat `feature/white-label-shops` work as isolated from `escrow-dev`/`dev-amn`; target `escrow-multi` for multi-shop deployment work.
|
||||
|
||||
221
09 - Audits/Mistral-Outsource-Package-2026-06-10.md
Normal file
221
09 - Audits/Mistral-Outsource-Package-2026-06-10.md
Normal file
@@ -0,0 +1,221 @@
|
||||
---
|
||||
title: Mistral Outsource Package — Audit Remediation 2026-06-10
|
||||
tags: [outsource, audit, remediation, mistral]
|
||||
created: 2026-06-10
|
||||
status: ready-to-send
|
||||
---
|
||||
|
||||
# Mistral Outsource Package — Audit Remediation 2026-06-10
|
||||
|
||||
Self-contained task package for an external AI agent (Mistral) working against the Amanat escrow codebase.
|
||||
|
||||
**Repo root:** `/Users/manwe/CascadeProjects/escrow`
|
||||
**Active branch for frontend/backend:** `feature/white-label-shops`
|
||||
**Active branch for scanner:** `development`
|
||||
**Active branch for deployment:** `main`
|
||||
|
||||
Each task is independent. Complete them in any order. Do not touch files outside the listed scope. Do not print secret values from `.env` files — reference only by variable name.
|
||||
|
||||
---
|
||||
|
||||
## Task 1 — M5: Scanner must fail startup when SCANNER_API_KEY is missing in production
|
||||
|
||||
**File:** `scanner/config.go`
|
||||
|
||||
**Context:** Lines 128–131 print a warning when `SCANNER_API_KEY` is empty but let the process start anyway. In production this means the scanner exposes all endpoints unauthenticated.
|
||||
|
||||
**What to do:**
|
||||
|
||||
Add an environment-gated hard-fail. If `SCANNER_API_KEY` is empty **and** `APP_ENV` is `production` (or `SCANNER_REQUIRE_AUTH=true`), call `log.Fatal(...)` / `os.Exit(1)` instead of `slog.Warn(...)`.
|
||||
|
||||
Keep existing dev-mode behaviour: if `APP_ENV` is not `production` and `SCANNER_REQUIRE_AUTH` is not `true`, keep the warn-only path.
|
||||
|
||||
Example shape (adapt to actual Go idioms used in the file):
|
||||
|
||||
```go
|
||||
if cfg.APIKey == "" {
|
||||
if os.Getenv("APP_ENV") == "production" || os.Getenv("SCANNER_REQUIRE_AUTH") == "true" {
|
||||
log.Fatal("[scanner] SCANNER_API_KEY must be set in production — refusing to start unauthenticated")
|
||||
}
|
||||
slog.Warn("[scanner] SCANNER_API_KEY is not set — all endpoints are unauthenticated (dev mode only)")
|
||||
}
|
||||
```
|
||||
|
||||
**Verification:** `go build ./...` and `go test ./...` must still pass.
|
||||
|
||||
---
|
||||
|
||||
## Task 2 — M4: Telegram debug panel must not show in production without explicit admin/developer role
|
||||
|
||||
**File:** `frontend/src/components/debug/telegram-debug-panel.tsx`
|
||||
|
||||
**Context:** Lines 44–56 set `showPanel = true` whenever `NODE_ENV !== 'production'` OR the page is opened inside a Telegram Mini App context OR `debug=1` / `amn-debug=1` is present in the URL/localStorage. This means the panel is always visible inside the Mini App in production, exposing user email, wallet address, internal API/socket URLs, and Telegram platform/version to any user.
|
||||
|
||||
**What to do:**
|
||||
|
||||
Extend the `showPanel` logic so that in production it only activates when **both** the debug request is present **and** the authenticated user has role `admin` or `developer`.
|
||||
|
||||
The component already has access to `user` from `useAuthContext()`:
|
||||
|
||||
```tsx
|
||||
const { user, authenticated, loading } = useAuthContext();
|
||||
```
|
||||
|
||||
Replace the `setShowPanel(...)` call inside the `useEffect` with:
|
||||
|
||||
```tsx
|
||||
const isPrivileged = user?.role === 'admin' || user?.role === 'developer';
|
||||
setShowPanel(
|
||||
process.env.NODE_ENV !== 'production' ||
|
||||
(nextContext.isMiniApp && isPrivileged && debugRequested) ||
|
||||
(debugRequested && isPrivileged)
|
||||
);
|
||||
```
|
||||
|
||||
Remove the bare `nextContext.isMiniApp` condition that shows the panel to all Mini App users.
|
||||
|
||||
Also update the initial `useState` default so it reads from user context properly — or just default to `false` and let the effect set it (safe since the effect runs on mount).
|
||||
|
||||
**Verification:** TypeScript compile (`npx tsc --noEmit`) must pass for this file.
|
||||
|
||||
---
|
||||
|
||||
## Task 3 — L1a: Remove hardcoded `password123` from deployment docker-compose files
|
||||
|
||||
**Files:**
|
||||
- `deployment/docker-compose.yml` line 152: `MONGO_INITDB_ROOT_PASSWORD=password123`
|
||||
- `deployment/dev-amn/docker-compose.yml` line 101: `MONGO_INITDB_ROOT_PASSWORD=password123`
|
||||
|
||||
**What to do:**
|
||||
|
||||
Replace each hardcoded `password123` with an env-var reference:
|
||||
|
||||
```yaml
|
||||
- MONGO_INITDB_ROOT_PASSWORD=${MONGO_INITDB_ROOT_PASSWORD:-changeme_local}
|
||||
```
|
||||
|
||||
Use `changeme_local` as the fallback, not `password123`, so it is obvious this is a placeholder that must be replaced in real deploys.
|
||||
|
||||
Add a comment above each block:
|
||||
|
||||
```yaml
|
||||
# Set MONGO_INITDB_ROOT_PASSWORD in your .env — default is local-only placeholder
|
||||
```
|
||||
|
||||
**Verification:** `docker compose config` must not error (dry-run parse only — do not start containers).
|
||||
|
||||
---
|
||||
|
||||
## Task 3b — L1b: Fix `undefined` password literals in migration SQL
|
||||
|
||||
**File:** `deployment/escrow-multi/migrate/migrations/0018_db_privilege_isolation.sql`
|
||||
|
||||
**Context:** Lines 11 and 19 create Postgres roles with `PASSWORD 'undefined'` — this is a literal string `undefined`, not a variable substitution. These were almost certainly written by accident.
|
||||
|
||||
**What to do:**
|
||||
|
||||
Replace the two `PASSWORD 'undefined'` literals:
|
||||
|
||||
```sql
|
||||
-- Before
|
||||
CREATE ROLE escrow_vital_user WITH LOGIN PASSWORD 'undefined';
|
||||
CREATE ROLE escrow_nonvital_user WITH LOGIN PASSWORD 'undefined';
|
||||
|
||||
-- After
|
||||
CREATE ROLE escrow_vital_user WITH LOGIN PASSWORD :'escrow_vital_password';
|
||||
CREATE ROLE escrow_nonvital_user WITH LOGIN PASSWORD :'escrow_nonvital_password';
|
||||
```
|
||||
|
||||
If psql variable syntax is not appropriate for the migration runner in use, use a clearly wrong placeholder value that can never accidentally work:
|
||||
|
||||
```sql
|
||||
CREATE ROLE escrow_vital_user WITH LOGIN PASSWORD 'REPLACE_ME_escrow_vital';
|
||||
CREATE ROLE escrow_nonvital_user WITH LOGIN PASSWORD 'REPLACE_ME_escrow_nonvital';
|
||||
```
|
||||
|
||||
Add a comment: `-- TODO: inject real passwords via migration runner env — do not commit real credentials`.
|
||||
|
||||
**Verification:** SQL must parse (`psql --dry-run` or equivalent syntax check).
|
||||
|
||||
---
|
||||
|
||||
## Task 4 — M6 Backend: Fix ESLint errors in backend (auto-fix pass + manual cleanup)
|
||||
|
||||
**Directory:** `backend/`
|
||||
|
||||
**Context:** `npm run lint` reports 29 errors including forbidden `require()` imports, empty catch blocks, and TypeScript namespace usage. 996 warnings exist but are lower priority.
|
||||
|
||||
**What to do:**
|
||||
|
||||
1. Run `cd backend && npm run lint -- --fix` to auto-fix all auto-fixable issues.
|
||||
|
||||
2. Manually fix remaining errors in these categories:
|
||||
- **Forbidden `require()` imports**: Replace `const x = require('y')` with `import x from 'y'` (or `import * as x from 'y'` for namespace imports). Do not change the runtime behaviour.
|
||||
- **Empty catch blocks**: Add a minimal comment `// intentional` or add `_err` as the parameter and log it if it looks like it should be logged. Do not silently swallow errors that would hide real bugs.
|
||||
- **TypeScript namespace usage**: If a `namespace Foo {}` can be a plain `module` or `interface`/`type` grouping, convert it. If the namespace is part of a declaration file or ambient module, keep it.
|
||||
|
||||
3. After manual fixes, run `npm run lint` again and confirm error count is 0 (warnings are acceptable).
|
||||
|
||||
4. Run `npm run typecheck` to ensure no regressions.
|
||||
|
||||
**Verification:** `npm run lint` exits 0 errors. `npm run typecheck` passes.
|
||||
|
||||
---
|
||||
|
||||
## Task 5 — M1: Remove `ignoreBuildErrors` from frontend Next.js config and fix resulting TS errors
|
||||
|
||||
**File:** `frontend/next.config.ts`
|
||||
|
||||
**Context:** Line 29 sets `typescript: { ignoreBuildErrors: true }`, masking type errors that reach production builds. The pre-push `tsc` hook is supposed to catch these, but production builds currently silently swallow them.
|
||||
|
||||
**What to do:**
|
||||
|
||||
Remove the `ignoreBuildErrors: true` line (or change to `ignoreBuildErrors: false`). Update the comment to reflect this:
|
||||
|
||||
```ts
|
||||
// TypeScript errors are caught here (Next.js build) and by the tsc-guard pre-push hook.
|
||||
typescript: { ignoreBuildErrors: false },
|
||||
```
|
||||
|
||||
Then run `npx yarn lint` and `npx tsc --noEmit -p tsconfig.json` inside `frontend/`. Fix any type errors that surface. Common patterns expected:
|
||||
|
||||
- Components with `@ts-nocheck` at the top — remove the suppression and fix the underlying type.
|
||||
- `any` casts that can be narrowed.
|
||||
- Missing `key` props on lists.
|
||||
|
||||
**Do not** fix type errors in payment or wallet components without reading the code carefully. If a type error in those files requires understanding complex payment domain logic, leave a `// TODO(audit): type error — needs domain review` comment and move on.
|
||||
|
||||
**Verification:** `npx tsc --noEmit` exits 0. `npx yarn build` completes without TypeScript errors.
|
||||
|
||||
---
|
||||
|
||||
## Task 6 — L2: Extend magic-byte validation to generic file upload routes
|
||||
|
||||
**Files:**
|
||||
- `backend/src/services/file/fileController.ts` — generic upload handler
|
||||
- `backend/src/services/file/chatAttachmentController.ts` (or similar) — reference: this file already has magic-byte validation
|
||||
|
||||
**Context:** The chat attachment upload path validates file magic bytes (file signatures) to ensure the actual content matches the declared MIME type. Generic uploads (product images, request templates, blog images) rely only on the MIME type declared by the client, which can be spoofed.
|
||||
|
||||
**What to do:**
|
||||
|
||||
1. Find the magic-byte validation function in the chat attachment controller. It likely reads the first N bytes of the upload buffer and compares against known signatures.
|
||||
|
||||
2. Extract or re-use that function in a shared utility: `backend/src/services/file/fileMagicBytes.ts` (or add it to `fileService.ts` if that's the right home).
|
||||
|
||||
3. Call the magic-byte check in `fileController.ts` for **all** upload routes that accept user-controlled files. Reject with HTTP 415 if the magic bytes do not match the declared MIME type.
|
||||
|
||||
4. Do not change the existing chat attachment path — it already works correctly.
|
||||
|
||||
**Verification:** `npm run typecheck` passes. Add or update a test in `backend/__tests__/` that uploads a file with a mismatched MIME/magic-byte pair and asserts HTTP 415.
|
||||
|
||||
---
|
||||
|
||||
## Notes for the executing agent
|
||||
|
||||
- **Never print** the contents of `.env`, `.env.dev`, or any variable containing `KEY`, `TOKEN`, `SECRET`, `PASSWORD`, or `PRIVATE`.
|
||||
- All changes must be on the branches specified at the top of this document.
|
||||
- Frontend changes: `feature/white-label-shops`. Backend changes: `feature/white-label-shops`. Scanner: `development`. Deployment: `main`.
|
||||
- Do not bump `package.json` version numbers — the orchestrating agent handles version bumps before any deploy.
|
||||
- Do not modify Woodpecker pipeline files, Dockerfiles, or CI configuration.
|
||||
- Each task's verification command must pass before marking the task done.
|
||||
88
09 - Audits/Workflow-Remediation-Plan-2026-06-10.md
Normal file
88
09 - Audits/Workflow-Remediation-Plan-2026-06-10.md
Normal file
@@ -0,0 +1,88 @@
|
||||
---
|
||||
title: Workflow Remediation Plan — 2026-06-10 Audit
|
||||
tags: [audit, workflow, plan, remediation]
|
||||
created: 2026-06-10
|
||||
status: draft
|
||||
---
|
||||
|
||||
# Workflow Remediation Plan — 2026-06-10 Audit
|
||||
|
||||
## Division of Labour
|
||||
|
||||
| Finding | Severity | Assignee | Rationale |
|
||||
|---|---|---|---|
|
||||
| C1 (secrets rotation) | Critical | Mistral → rotation doc (Haiku writes checklist) | Rotation is human action; doc is mechanical |
|
||||
| C2 (LLM proxy auth) | Critical | Sonnet | Auth pattern integration needs codebase knowledge |
|
||||
| H1 (bot claim URL) | High | Haiku | Mechanical serializer split — no domain logic |
|
||||
| H2 (file ownership) | High | Sonnet | Needs to read ownership model from DB schema |
|
||||
| H3 (oracle quoting) | High | Sonnet (grouped with H4+M3) | Same file, complex payment logic |
|
||||
| H4 (UUID/JWT mismatch) | High | Sonnet (grouped with H3+M3) | Same file, identity normalization |
|
||||
| M3 (permit relay) | Medium | Sonnet (grouped with H3+H4) | Same file, rate-limit implementation |
|
||||
| M4 (debug panel) | Medium | Mistral | Simple role-gating change |
|
||||
| M5 (scanner startup) | Medium | Mistral | One Go startup guard |
|
||||
| M6 (lint errors) | Medium | Mistral | Auto-fix pass + manual cleanup |
|
||||
| L1 (deployment defaults) | Low | Mistral | Replace hardcoded strings |
|
||||
| L2 (MIME hardening) | Low | Mistral | Reuse existing magic-byte validator |
|
||||
| M1 (ignoreBuildErrors) | Medium | Mistral | Config change + TS cleanup |
|
||||
|
||||
## Workflow Phase Design
|
||||
|
||||
### Phase 1 — Haiku (parallel)
|
||||
Two agents run simultaneously:
|
||||
|
||||
**H1-fix**: `tenantBotService.ts`
|
||||
- Create `toPublicBotList()` — identical to `toPublicBot()` but always returns `claimUrl: null`
|
||||
- Replace usage in the list/map path with the new function
|
||||
- Keep `toPublicBot()` for the dedicated claim-link endpoint
|
||||
|
||||
**C1-doc**: Write `C1-Secrets-Rotation-Checklist-2026-06-10.md`
|
||||
- Rotation steps per category (env files, test fixtures, docs)
|
||||
- History cleanup instructions (git filter-repo, coordinate clones)
|
||||
- Prevention checklist (gitleaks hook, CI scan)
|
||||
|
||||
### Phase 2 — Sonnet (parallel, non-overlapping files)
|
||||
Three agents run simultaneously:
|
||||
|
||||
**C2-fix**: `frontend/src/app/api/llm/route.ts` + `amanat-assist/llm-proxy/index.mjs`
|
||||
- Add session/JWT auth check to the Next.js route (401 if not authenticated)
|
||||
- Add 64KB body size guard to route
|
||||
- Flip CORS default from wildcard to closed in proxy
|
||||
- Add 256KB body cap to proxy
|
||||
- Restrict provider to ALLOWED_PROVIDERS env var
|
||||
- Redact error logging (status + truncated message only)
|
||||
|
||||
**H2-fix**: `backend/src/services/file/fileController.ts` + `fileRoutes.ts`
|
||||
- Read ownership model from upload code to understand user → file path mapping
|
||||
- Add ownership check before delete: file must belong to user or user must be admin
|
||||
- Add ownership check before info: same rule
|
||||
- Return 403 on unauthorized access
|
||||
|
||||
**Payment-fix** (H3 + H4 + M3 combined — single agent to avoid same-file conflicts):
|
||||
- H3: Remove `ORACLE_QUOTING_ENABLED` flag-gated fallback; always use server-side oracle path; fail 422 if offer not loadable
|
||||
- H4: Replace raw `payment.buyerId !== userId.toString()` comparisons with canonical helper that checks both legacy ObjectId and pgId UUID (3 sites in `requestNetworkRoutes.ts` + 3 in `paymentRoutes.ts`)
|
||||
- M3: Add buyer ownership check to permit relay route; add in-memory rate limiter (5 relay attempts/payment/minute)
|
||||
|
||||
### Phase 3 — Haiku (parallel verification)
|
||||
- `cd backend && npx tsc --noEmit -p tsconfig.json` — report pass/fail + errors
|
||||
- `cd scanner && go build ./...` — report pass/fail
|
||||
|
||||
### Phase 4 — Opus (final review)
|
||||
Read all 6 changed files, assess:
|
||||
- Is each fix correct and complete?
|
||||
- Are there bypass vectors?
|
||||
- Regressions in legitimate flows?
|
||||
- TypeScript type safety?
|
||||
Return a structured PASS/NEEDS_FIX verdict per file + overall READY/NEEDS_WORK.
|
||||
|
||||
## Findings NOT covered by this workflow (human action required)
|
||||
|
||||
- **C1 rotation**: The checklist is generated, but actual key rotation is a human action (BotFather, provider dashboards, re-deployment with new values, then git history rewrite after rotation confirmed).
|
||||
- **H5 dependencies**: Upgrade lockfiles needs careful testing — separate controlled branch recommended.
|
||||
- **M2 browser tokens**: Moving to httpOnly cookies is a large auth refactor — tracked as a separate initiative.
|
||||
|
||||
## Estimated output
|
||||
|
||||
- ~6 file edits across frontend, backend, amanat-assist
|
||||
- 1 new doc (C1 rotation checklist)
|
||||
- Typecheck passes expected (Opus review will catch regressions if any)
|
||||
- Backend tsc was already passing before this workflow — must stay passing
|
||||
Reference in New Issue
Block a user