142 lines
15 KiB
Markdown
142 lines
15 KiB
Markdown
# Problems
|
||
|
||
Findings from systematic bug-hunt and test expansion (May 2026).
|
||
|
||
**Verification:** `test.bat` from project root, or `go test ./...` in `server`/`agent` and `npm test` in `server/web`. AI handler only: `cd server && go test ./internal/api/... -run AI -v`.
|
||
|
||
---
|
||
|
||
## Open
|
||
|
||
### Critical / security
|
||
|
||
- [CRITICAL] **server/internal/api/router.go** — Unauthenticated build downloads (`/api/v1/builds/{id}/download`, `/artifact/`). Intentional for agent reinstall (UUID is secret). Suggested fix: optional auth toggle or short-lived signed URLs.
|
||
- [CRITICAL] **server/internal/api/websocket.go** — Agent WebSocket (`/ws/agent`) accepts connections without fleet secret when server secret is unset (first-run). With secret configured, bad secret is rejected; agent can still pick any `agent_id`. Suggested fix: bind `agent_id` to fleet secret baked into forged binary (S2 from prior audit).
|
||
- [CRITICAL] **server/internal/api/ai_handler.go** — Unauthenticated AI endpoints (`/agent/decide`, `/report`, `/heartbeat`); SSRF via caller-supplied Ollama URL. Suggested fix: require fleet secret or dashboard auth.
|
||
- [HIGH] **server/internal/api/router.go** — Plaintext passwords in `users.json`; any authed user can POST `/users`. Suggested fix: bcrypt-only storage (partially done), restrict user management to admin role.
|
||
- [HIGH] **server/internal/api/fleet_handler.go** — Remote code execution via authenticated API (`powershell`/`exec`/`upload`). By design — treat dashboard login as root.
|
||
|
||
### High
|
||
|
||
- [HIGH] **server/internal/api/fleet_handler.go** — `upload_log` returns content in tool report only; no dedicated log ingest API.
|
||
- [HIGH] **agent/** — `AutoSpread` runs when baked `true` in forge; dangerous if enabled on non-owned fleets.
|
||
- [HIGH] **agent/** — Process hollowing with `-tags hollow` + forge flag; bounds/reloc issues in `hollow_windows.go`.
|
||
|
||
### Medium / UX
|
||
|
||
- [MEDIUM] **server/web** — Compact agent list default; expand on click.
|
||
- [MEDIUM] **server/web** — Remote actions disabled unless `online` (by design).
|
||
- [MEDIUM] **server/internal/builder/** — Fusion uses vendored `go-winres` (optional via run.bat).
|
||
- [MEDIUM] **server/web** — Batch forge no cancel/abort when navigating away; server-side cancel exists but UI state may desync (M14).
|
||
- [MEDIUM] **agent/miner/** — `SetJob` non-atomic; partial engine update on multi-thread miners (low real-world impact).
|
||
- [MEDIUM] **server/internal/db/sqlite.go** — ~~`SetPinnedBuild` with unknown id unpins all builds then pins nothing.~~ Fixed: returns `build not found`; `TestSetPinnedBuildUnknownID`.
|
||
- [MEDIUM] **server/web/src/pages/AgentsPage.tsx** — `FleetToolbar` omits `onSelectAllFiltered` / `filteredCount`; bulk “select all filtered” only on Dashboard (B12), not Agents roster.
|
||
|
||
### Low
|
||
|
||
- [LOW] **server/internal/api/fleet_handler.go** — Package-global `xmrPriceCache` shared across requests/tests; no per-server isolation if multiple routers in one process (unlikely in production).
|
||
- [LOW] **server/web/src/types/ws.ts + server/internal/api/ws_types.go** — WS payloads typed in two places; drift risk.
|
||
- [LOW] **tests/** — No integration tests for remote actions end-to-end.
|
||
- [LOW] **agent/** — Mesh P2P requires build tag `p2p`.
|
||
- [LOW] **server/config.go** — `LoadConfig` uses legacy `mergeConfig` (not `mergeConfigExplicit`); a hand-edited `config.json` omitting bool fields can still zero them on restart.
|
||
- [LOW] **server/internal/api/config_handler.go** — `ConfigHandler.db` is unused; handler delegates entirely to `ConfigProvider`.
|
||
- [LOW] **server/main.go** — `UpdateConfigFromJSON` has no semantic validation (negative ports, empty pool host, etc.); invalid values persist to disk.
|
||
- [LOW] **server/internal/db/agent_meta.go** — `decodeTags` silently drops invalid JSON in `tags` column (corrupt values become empty slice).
|
||
- [LOW] **server/web/e2e/smoke.spec.ts** — E2E login still uses hardcoded `drjones`/`czapiewski`; fails against first-run random `admin` password. Suggested fix: seed `users.json` in E2E fixture or read creds from env.
|
||
- [LOW] **server/web/src/types/index.ts** — Interfaces only; no runtime type guards for API JSON (validation ad hoc in components).
|
||
- [LOW] **server/web/src/api/client.ts** — `estimateFusion` requires `prepFile` but has no client-side guard (unlike `buildAgent` fusion path); server returns error if missing.
|
||
- [LOW] **server/web/src/pages/SettingsPage.tsx** — Calibrate UI lives here (`/settings` route); no separate `CalibratePage.tsx`. Form labels lack `htmlFor` — a11y follow-up.
|
||
- [LOW] **server/internal/maintenance/retention.go** — `os.RemoveAll` errors ignored; failed disk cleanup is silent.
|
||
- [LOW] **server/internal/maintenance/retention.go** — Artifact dir removed before `DeleteBuild`; if DB delete fails, build row remains without files on disk.
|
||
- [LOW] **server/internal/maintenance/retention.go** — `StartRetentionJobs` goroutine has no shutdown hook (acceptable for server process lifetime).
|
||
|
||
### Untested packages (next coverage targets)
|
||
|
||
- [LOW] **server/internal/builder/** — compile/fusion/disguise paths still mostly integration-only (estimate/handler/platform covered).
|
||
- [LOW] **server/internal/api/** — `agent_config.go`, `server_policy.go` lack dedicated unit tests (covered indirectly via router/integration).
|
||
- [LOW] **agent/stats/** — platform reporters (Windows/Linux/Darwin) untested.
|
||
- [LOW] **agent/deploy/** — autospread, hollow, NAT punch, tunnel — platform/integration only (`common`/`identity`/`spreadkit` helpers now tested).
|
||
- [LOW] **agent/client/** — `client.go`, platform commands/posture probes untested (protocol/posture/resource pressure covered).
|
||
- [LOW] **server/web/src/components/Charts/GaugeRing.tsx** — Center label uses raw `value` while SVG arc clamps to 0–100%; negative/over-max inputs show misleading text (e.g. `200%`).
|
||
- [LOW] **server/web/src/help/settingHelp.ts** — `FIELD_HELP.wallet` still says "~95 characters"; validator accepts 90–106 (same drift fixed in forgeCompatibility / cheatSheetContent troubleshoot).
|
||
|
||
---
|
||
|
||
## Fixed (this session)
|
||
|
||
- **server/internal/models/** — Added `agent_test.go` (10 tests): JSON round-trips for all exported structs; omitempty/minimal decode.
|
||
- **server/internal/ollama/** — Added `engine_test.go` (14 tests): `NewEngine` defaults, type JSON round-trips, mock decide/health paths, markdown JSON extraction, error branches.
|
||
- **server/internal/sys/** — Added `firewall_test.go` (2 tests): invalid port; non-Windows stub error.
|
||
- **server/internal/alerts/** — Added `notify_test.go` (6 tests): Telegram/email no-op paths, SMTP defaults, `NotifyAll` no-panic.
|
||
- **server/internal/pool/** — Added `manager_test.go` (8 tests): validation, `poolKey`, status levels, setters, `ListStatus`.
|
||
- **server/internal/db/sqlite.go** — `SetPinnedBuild` returns error when id not found; `TestSetPinnedBuildUnknownID`.
|
||
- **agent/client/** — Added `protocol_test.go`, `posture_types_test.go`, `resource_pressure_test.go` (25 tests).
|
||
- **agent/client/client.go** — Empty `"error"` in job payload no longer treated as server error.
|
||
- **agent/config/** — Added `schedule_test.go` (6 tests): mining mode, clock parse, schedule windows.
|
||
- **agent/deploy/** — Added `common_test.go`, `identity_test.go` (18 tests): naming, install paths, agent ID lifecycle.
|
||
- **agent/job/** — Added `job_test.go` (2 tests): JSON round-trip.
|
||
- **server/internal/api/websocket.go** — `checkDashboardWSToken` compared plain password to bcrypt hash; dashboard WS auth failed after user migration. Now uses `checkPassword`.
|
||
- **server/internal/api/** — Added unit/integration tests for remaining handlers: `handlers.go` (agent/build REST), `router.go` (auth middleware, users, rotate-secret, SPA/dropper routes), `websocket.go` (agent/dashboard WS, fleet secret, max agents, log tail), `dropper_handler.go`, `blueprint_handler.go`, `ws_types.go`. New files: `router_test.go`, `dropper_handler_test.go`, `blueprint_handler_test.go`, `websocket_test.go`, `ws_types_test.go`; expanded `handlers_test.go`. `go test ./internal/api/...` — 159 tests PASS.
|
||
- **server/web/src/components/** — Added `components.test.tsx` (57 tests) covering all 22 component TSX modules (NeonCard, HelpTip, downloads, ErrorBoundary, SessionGate, charts, fleet panels/toolbar/list/remote actions, forge hints, visual widgets, layout, ambient/matrix/cursor). Vitest `environmentMatchGlobs` includes `src/components/**`.
|
||
- **server/web/src/pages/AgentsPage.test.tsx** — `AgentRemoteActions` mocked to avoid live `listBuilds` / ECONNREFUSED :3000 in detail-panel tests.
|
||
- **server/web/src/api/client.ts** — `fetchJSON` spread `...options` after merged headers could drop `Content-Type` and `Authorization` when callers pass `options.headers`; headers now merged after rest spread.
|
||
- **server/web/src/api/** — Added `client.test.ts` (19) and `download.test.ts` (6): paths, query params, auth headers, FormData fusion builds, error bodies. Expanded `auth.test.ts` (+1 sessionStorage throw path).
|
||
- **server/web/src/context/** — Added `WebSocketContext.test.tsx` (2), `WebSocketProvider.test.tsx` (8), `ForgeContext.test.tsx` (4): mock WebSocket connect URL/token, message handlers, `_seq` ring buffer, reconnect timer, forge state machine.
|
||
- **server/web/src/pages/BuilderPage.tsx** — Load failure no longer stuck on “Loading forge defaults…” when `form` is null; error message shown instead. Wallet placeholder/short-wallet hint aligned to 90–106 chars. Exported `formatBytes` helper.
|
||
- **server/web/src/pages/SettingsPage.tsx** — Wallet placeholder aligned to 90–106 chars. Exported `deepMerge` helper (config import).
|
||
- **server/web/src/pages/** — Added `BuilderPage.test.tsx` (13) and `SettingsPage.test.tsx` (11, Calibrate UI at `/settings`). Page suite now 4 files / 46 tests.
|
||
- **server/web/src/test/fixtures.ts** — Added `mockServerConfig()` for page/API tests.
|
||
- **server/internal/api/config_handler.go** — PUT errors return valid JSON; `invalid config:` maps to HTTP 400; GET sets explicit 200.
|
||
- **server/config.go** — `mergeConfigExplicit` tracks nested key presence; partial PUT `{"server":{"dashboard_subtitle":"x"}}` no longer resets sibling booleans (H14 nested shallow-merge).
|
||
- **server/internal/api/config_handler_test.go** — 10 handler unit tests (GET/PUT, 405, invalid JSON, 400/500 paths, JSON escaping).
|
||
- **server/config_test.go** — 9 `mergeConfigExplicit` regression tests (partial PUT, nested merge, defaults, bool false, fallback).
|
||
- **server/internal/maintenance/** — Added `retention_test.go` (12 tests): `StartRetentionJobs` no-op/disabled, immediate run, 6h tick interval, stats/build purge via temp sqlite + filesystem, zero-retention skips, closed-DB error logs, combined stats+builds pass. Coverage ~97%. Exported `retentionTickInterval` + `runRetentionFn` hooks for testability only.
|
||
- **server/web/src/pages/AgentsPage.tsx** — Bulk command errors now alert user (parity with Dashboard B13).
|
||
- **server/web/src/pages/DashboardPage.tsx** — Share log table uses composite React key when `share.id` absent; exported `formatShareTime` helper.
|
||
- **server/web/src/pages/AgentsPage.tsx** — `listAgents` no longer overwrites live WS agent list when socket already connected (`isConnectedRef` guard).
|
||
- **server/web/src/pages/** — Added `DashboardPage.test.tsx` (11) and `AgentsPage.test.tsx` (11); vitest config extended for `.tsx` + `@testing-library/react`.
|
||
- **server/internal/db/retention.go** — `ListBuildsOlderThan` used a partial column list; now uses `buildSelectCols` + `scanBuild` for consistent full `BuildRecord` fields.
|
||
- **server/internal/api/integration_test.go** — Integration tests used stale hardcoded `drjones`/`czapiewski` credentials; server now generates random `admin` password on first run. Tests seed deterministic `users.json` before router init.
|
||
- **server/web/src/help/fleetAnalytics.ts** — `contributionBars` included offline agents in total hashrate denominator, skewing contribution percentages on the dashboard.
|
||
- **server/web/src/pages/SettingsPage.tsx** — Access Control help text still referenced removed default credentials; updated to describe first-run console password.
|
||
- **server/internal/api/ai_handler_test.go** — Expanded unit tests for `HandleDecide`, `HandleReport`, `HandleHeartbeat`, engine lifecycle, numeric constants (1000 report cap, 60s heartbeat, 120-char reasoning truncate), Ollama-failure sleep fallback, event broadcaster, `recordActivity` merge.
|
||
- **server/internal/api/fleet_handler_test.go** — Unit tests for all exported `FleetHandler` methods (`GetAlerts`, `GetPoolStatus`, `GetAIActivity`, `GetXMRPrice`, `GetEarnings`/`GetEarningsEstimate`, `GetAgentLog`, `PostAgentCommand`, `PutAgentMeta`, `PostBulkCommand`), `EstimateXMRPerDay`/`parseFloatQuery`, earnings/XMR price cache TTLs, SupportXMR field normalization, HTTP error branches (503/502/400), and WS command paths via mock transport + test agent WS.
|
||
- **server/web/src/help/forgeCompatibility.ts** — Wallet preflight message said length 95–106 but validator accepts 90–106; message aligned with `looksLikeXMRWallet()`.
|
||
- **server/web/src/help/** — Added/expanded vitest coverage: `forgeCompatibility.test.ts` (37), `forgeRules.test.ts` (46), `settingHelp.test.ts` (8).
|
||
- **server/web/src/help/buildManager.test.ts** — 9 tests: `blueprintDiff` (added/removed/changed, sort, nested, arrays, empty), `buildRequestFromRecord` merge/override.
|
||
- **server/web/src/help/cheatSheetContent.test.ts** — 19 tests: pipeline/network/fusion/AI guides, `FORGE_VS_CALIBRATE`, `TROUBLESHOOTING`, `ROADMAP_FEATURES`, `CHEAT_SECTIONS` registry.
|
||
- **server/web/src/help/forgeDefaults.test.ts** — 7 tests: `FORGE_BUILD_DEFAULTS` shape, `forgeDefaultsFromServer` public URL / pool / sign / obfuscate.
|
||
- **server/web/src/help/remoteActions.test.ts** — Expanded to 11 tests: `aggressiveActionHint`, spread/mesh gating, legacy undefined caps.
|
||
- **server/web/src/help/cheatSheetContent.ts** — Troubleshooting "Shares all rejected" wallet text aligned to 90–106 chars (was stale "95 chars").
|
||
- **server/web/src/types/index.test.ts** — Structural fixture tests for all major exported interfaces (20 tests); documents no runtime type guards.
|
||
|
||
---
|
||
|
||
## Fixed (earlier passes)
|
||
|
||
See git history and prior audit IDs (B1–B42, C1–C6, H1–H8, etc.) in README / tests/README.md.
|
||
|
||
---
|
||
|
||
## Recommended next section
|
||
|
||
1. **server/web/src/help/settingHelp.ts** — align wallet help text to 90–106 chars
|
||
2. **server/web/e2e/smoke.spec.ts** — seed first-run admin creds for E2E
|
||
3. **agent/stats/** + **agent/deploy/** integration paths — platform reporters, autospread/hollow
|
||
4. **Agent WS token auth** (S2) — security hardening
|
||
|
||
---
|
||
|
||
## Test run snapshot (this session)
|
||
|
||
| Suite | Result |
|
||
|-------|--------|
|
||
| `server/internal/api/...` (full) | PASS (159 tests) |
|
||
| `server` Go tests | PASS (all packages incl. models, ollama, sys, alerts, pool) |
|
||
| `agent` Go tests | PASS (client, config, deploy, job, miner) |
|
||
| `server/web` vitest (page tests) | PASS — 4 files, 46 tests |
|
||
| `server/web` vitest (api/context/hooks) | PASS — 6 files, 43 tests |
|
||
| `server/web` vitest (`components.test.tsx`) | PASS — 1 file, 57 tests |
|
||
| `server/web` vitest (full suite) | PASS — 28 files, 347 tests |
|
||
| `server/web` vitest (`src/help/`) | PASS — 16 files, 181 tests |
|