Triage full-eval findings into Current state (work queue / known debt / deferred)
This commit is contained in:
@@ -136,9 +136,36 @@ this. When unsure whether a change is contract-affecting, assume it is and check
|
||||
- **Never edit a `startos/versions/<v>.ts` that's already been built/installed** — add a new version file.
|
||||
- **Don't push to GitHub by default** — remote is self-hosted Gitea.
|
||||
|
||||
## Current state — git caught up; box, tree, and git aligned at `0.2.124`
|
||||
## Current state — full eval done (2026-06-13); findings triaged below
|
||||
|
||||
- **Box, local tree, and git are now aligned at relay `0.2.124`** (app at `0.2.155`). `startos/versions/index.ts` `current: v_0_2_124`; the StartOS dashboard reflects the same.
|
||||
- **Git caught up (2026-06-13).** Everything from `v0.2.12` → `v0.2.124` — previously uncommitted on top of `b7f7590 v0.2.11` — is now committed as 7 logical chunks (`705807e`…`fb11dd6`): internal-meetings pipeline + speaker tools, Spark Control hardware backend, billing (tiers / credits / BTCPay / Zaprite), TTS backends, core routing / identity / dashboard, the v0.2.12→0.2.124 packaging + version graph, and the agent-docs split. **Working tree is clean.** No git remote is configured (local history only).
|
||||
- **Post-hoc speaker tools are live**: `meeting-speaker-edits.js` (merge / recluster / repolish + backfill) and the matching `PATCH/POST /admin/internal-meetings/:id/{merge-speakers,recluster,repolish}` routes; the dashboard exposes the controls. Tests pass via `cd server && npm test` (47 tests).
|
||||
- **Next:** the ROADMAP "commit the uncommitted tree" item is DONE; remaining work is the speaker-tool follow-ups and the empty-analysis-section issue (see ROADMAP.md / docs/issues-backlog.md).
|
||||
- **Box, local tree, and git aligned at relay `0.2.124`** (app at `0.2.155`). `startos/versions/index.ts` `current: v_0_2_124`. Git history is local-only (no remote). Working tree is clean apart from an untracked `server/package-lock.json` left by the eval's `npm install` — a generated artifact, intentionally NOT committed.
|
||||
- **Full independent evaluation run 2026-06-13** (evaluator + security-auditor + exerciser + doc-auditor + start9-spec-checker). Report committed at `EVALUATION.md` (`b08e836`); it's overwritten in place each run so re-running gives a reviewable diff. 47/47 tests still pass; server boots clean. Findings triaged into the three buckets below.
|
||||
- **Post-hoc speaker tools remain live**: `meeting-speaker-edits.js` (merge / recluster / repolish + backfill) + the `PATCH/POST /admin/internal-meetings/:id/{merge-speakers,recluster,repolish}` routes; dashboard exposes the controls.
|
||||
|
||||
### Work queue — P0/P1 (fix first)
|
||||
|
||||
1. **SSRF on `/relay/transcribe-url` + `/relay/summarize-url`** — `downloadDirect` (`server/routes/transcribe-url.js:99-159`) fetches any caller `media_url` with redirect-follow and no scheme/host allowlist; reachable via a self-chosen `X-Recap-Install-Id` (effectively unauthenticated). Reject non-http(s) + private/loopback/link-local hosts; re-validate per redirect hop. *(evaluator + security-auditor + exerciser — headline risk)*
|
||||
2. **Billing money-leak: early/mid-cycle renewal resets `monthly_consumed = 0`** — `extendUserTier` → `setUserTier` (`server/credits.js:770→641`) hands back a full monthly allotment for free; a webhook double-fire across restart compounds it. Preserve `monthly_consumed`/`last_renewal_at`, roll only on a true period boundary; add a money-path test (`tier-expiry.test.js:63` asserts only the date today). *(evaluator)*
|
||||
3. **`multer@1.4.5-lts.2` DoS CVEs** (CVE-2025-47944/47935/48997/7338) — malformed multipart can crash the process. Upgrade to `^2.0.1`, re-test the `file` upload field (`server/package.json:15`). *(security-auditor + exerciser)*
|
||||
|
||||
### Known debt — P2 (accepted for now; fix opportunistically)
|
||||
|
||||
- Path traversal on internal-meetings `:id` (admin-gated): validate `^[A-Za-z0-9_-]+$` before `path.join` — `routes/internal-meetings.js:84,91,242` (`output-store.js:52` shows the pattern). *(security-auditor + exerciser)*
|
||||
- Non-constant-time operator-key compare (`!==`) on `relay_cloud_operator_key` — `server/identity.js:43,84`; use `timingSafeEqual` like the admin path. *(evaluator + security-auditor)*
|
||||
- In-memory webhook dedup Set lost on restart → double-credit/double-extend — `routes/credits.js:63`, `zaprite-webhook.js:27`; persist processed invoice/order ids. *(security-auditor)*
|
||||
- Malformed JSON body → full Node stack trace (FS paths) — add an Express `entity.parse.failed` → JSON-400 handler. *(exerciser)*
|
||||
- BTCPay declared `optional:false`/`kind:'running'` despite "optional" comments → StartOS won't start the relay without BTCPay co-installed — `startos/manifest/index.ts:38-49`, `startos/dependencies.ts`. Decide, then make manifest + dependencies + comment agree. *(start9-spec-checker)*
|
||||
- No money-path unit tests (`commitCredit`/`refundCredit`/`applyTierPromotion`/`planBackend`/grant handlers) — why the P1 billing bug ships green. *(evaluator)*
|
||||
- `routes/internal-meetings.js` is 2225 lines; extract the MD/HTML formatters + storage/backfill layer. *(evaluator)*
|
||||
- Fully-open `cors()` incl. `/admin/*` — scope origins — `server/index.js:54`. *(evaluator)*
|
||||
- Doc drift: AGENTS.md "Stack" line mis-states `/relay/*` auth (most routes are per-call header auth; only `routes/user-tier.js` needs the operator key); the admin-exempt list omits `/admin/btcpay/callback` (`admin-auth.js:70`). *(doc-auditor)*
|
||||
|
||||
### Deferred — P3+ (later decision or bulk cleanup)
|
||||
|
||||
- Security hardening: no `/relay/*` rate limiting; container likely runs as root (entrypoint `chown`s uid 1001 but no `USER` directive); dashboard `innerHTML` stored-XSS surface; `lan-fetch` TLS verify off (admin-set URL only); debug/error fields leaked to clients. *(security-auditor + evaluator)*
|
||||
- Packaging/ops: prune the 126 `startos/versions/*.ts` files; pin `yt-dlp` in the Dockerfile; Dockerfile per-subdir `COPY` footgun; manifest polish (SPDX license, `docsUrls`, real repo URLs, icon format); no `README.md` (blocks public-registry submission only — moot for this private box). *(start9-spec-checker + evaluator)*
|
||||
- `/relay/health` reports stale `0.2.11` — `server/package.json` never bumped past 0.2.11; bump it to track the StartOS version. *(exerciser + doc-auditor)*
|
||||
- Doc fixes (bulk): the `test/` layout lists 3 of 6 files; `server/index.js:3-6` "two endpoints" header comment is stale; `POST /admin/logout` undocumented. *(doc-auditor)*
|
||||
- Untested blind spot: the live upload → merge → recluster → repolish pipeline (admin-gated + needs Spark Control) has only unit coverage; the dependency audit ran offline — re-run `npm audit`/`osv-scanner` with network to confirm the multer finding and catch transitive CVEs. *(all agents)*
|
||||
|
||||
**Pre-existing backlog** (separate from the eval): speaker-tool follow-ups and the empty-analysis-section issue — see `ROADMAP.md` / `docs/issues-backlog.md`.
|
||||
|
||||
Reference in New Issue
Block a user