ea6f54ad03
Adds hundreds of unit/integration/e2e tests, fixes WS bcrypt auth, config merge, fleet analytics, agent schedule/log tail, and documents stale PROBLEMS items. Updates PROBLEMS.md, README, and test scripts; ignores local spread-kits and coverage dirs.
151 lines
18 KiB
Markdown
151 lines
18 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
|
||
|
||
- [HIGH] **server/internal/api/fleet_handler.go** — Remote code execution via authenticated API (`powershell`/`exec`/`upload`). By design — treat dashboard login as root.
|
||
|
||
### Untested packages (next coverage targets)
|
||
|
||
- [LOW] **server/internal/builder/** — Full compile/fusion/disguise still need integration (requires go/garble/fusion source on host); unit tests cover ~110 pure-helper paths.
|
||
- [LOW] **server/internal/api/** — `agent_config.go`, `server_policy.go` lack dedicated unit tests (covered indirectly via router/integration).
|
||
- [LOW] **agent/stats/** — Per-OS memory/CPU internals still integration-only.
|
||
- [LOW] **agent/deploy/** — Live SSDP/SMB/SSH/cloudflared still integration-only.
|
||
- [LOW] **agent/client/** — Live WS/commands/posture probes still integration-only.
|
||
- [LOW] **agent/miner/** — `engine.go` (RandomX), `pool.go` worker/resource guard, `stratum.go` TCP login/submit loop — integration-only.
|
||
- [LOW] **server/web/src/types/ws.ts + server/internal/api/ws_types.go** — WS payloads typed in two places; drift risk. (Acceptable — no runtime impact.)
|
||
- [LOW] **server/internal/maintenance/retention.go** — `StartRetentionJobs` goroutine has no shutdown hook. (Acceptable for server process lifetime.)
|
||
|
||
---
|
||
|
||
## Documented / by design
|
||
|
||
Findings reclassified after code verification (May 2026). Not bugs — documented in README, tests/README, help text, or tests.
|
||
|
||
| Was | Resolution |
|
||
|-----|------------|
|
||
| [HIGH] Plaintext passwords in `users.json` | `users.json` stores bcrypt hashes (cost 12). Legacy plaintext auto-migrates on startup and login via `checkPassword` in `router.go`. Documented in README Security + First-run login. |
|
||
| [HIGH] `upload_log` no dedicated ingest API | By design: logs via `get_log` command / Fetch Log UI and AI `upload_log` tool reports. Documented in README Fleet Roster, tests/README, Field Guide tips. |
|
||
| [MEDIUM] Compact list / expand on click | Implemented in `AgentListItem.tsx` (`compact-row`, click toggles expand). Documented in README Fleet Roster UX. |
|
||
| [MEDIUM] Remote actions disabled when offline | Intentional — requires live WebSocket. Documented in README, `settingHelp.ts`, Field Guide tips. Vitest + Playwright `e2e/remote-actions.spec.ts`. |
|
||
| [MEDIUM] Fusion uses vendored go-winres | Optional tool; `run.bat` installs, builder uses `go run github.com/tc-hib/go-winres`. Documented in README Forge section. |
|
||
| [LOW] SettingsPage = Calibrate / no CalibratePage | Nav label **Calibrate** → route `/settings` → `SettingsPage`. Navigation table in README. (A11y `htmlFor` remains in Open.) |
|
||
| [LOW] `types/index.ts` no runtime guards | Compile-time contracts only; comment block at top of file + tests/README Architecture note. |
|
||
| [LOW] No e2e for remote actions | Added `server/web/e2e/remote-actions.spec.ts` (offline agent mock → disabled buttons). Vitest coverage in `components.test.tsx`. |
|
||
| [LOW] Mesh P2P needs `p2p` tag | Forge adds `-tags p2p` when mesh enabled (`compile.go`); manual builds documented in README agent section. |
|
||
|
||
---
|
||
|
||
## Fixed (this session — May 2026 full pass)
|
||
|
||
- **[MEDIUM] server/web/src/pages/BuilderPage.tsx** — Added `useEffect` cleanup on unmount that calls `api.cancelBuild(cancelTokenRef.current)` and sets `batchCancelRef.current = true`. Navigating away from the Forge page now cancels any in-progress server-side compile (M14 UI desync closed).
|
||
- **[MEDIUM] agent/miner/pool.go** — Added atomic `jobGen` counter incremented in `SetJob`. Workers snapshot `jobGen` before each 256-nonce inner loop and break early when it changes, eliminating the "stale batch" window. Engine updates moved outside the pool write-lock (each `Engine` has its own `RWMutex`).
|
||
- **[LOW] server/internal/api/fleet_handler.go** — `xmrPriceCache` moved from package-global vars (`xmrPriceMu`, `xmrPriceCache`) into `FleetHandler` struct fields (`xmrPriceMu`, `xmrPriceCache`). Multiple routers in one process no longer share a stale cache. Tests updated.
|
||
- **[LOW] server/config.go** — `LoadConfig` now calls `mergeConfigExplicit` (with a key-presence map) instead of legacy `mergeConfig`. Boolean fields absent from a hand-edited `config.json` now keep `DefaultConfig` values rather than being zeroed on restart.
|
||
- **[LOW] server/internal/api/config_handler.go** — Removed unused `db *db.Database` field from `ConfigHandler` and updated `NewConfigHandler` signature. All call sites updated (`router_test.go`, `integration_test.go`, `config_handler_test.go`, `main.go`).
|
||
- **[LOW] server/main.go** — `UpdateConfigFromJSON` now validates semantic constraints before merging: port ranges 1–65535, pool port range, non-negative max_agents/stats_retention_hours/build_retention_days/max_build_size_mb. Invalid values return `"invalid config: …"` 400 without touching disk.
|
||
- **[LOW] server/internal/db/agent_meta.go** — `decodeTags` now logs corrupt tag JSON via `log.Printf` instead of silently discarding it.
|
||
- **[LOW] server/web/src/pages/SettingsPage.tsx** — All `<label>` elements paired with text inputs now carry `htmlFor` attributes matching corresponding `id` attributes on their inputs (33 label/input pairs). A11y issue closed.
|
||
|
||
## Fixed (this session — May 2026 security pass)
|
||
|
||
- **[CRITICAL] server/internal/api/router.go** — Build download/artifact/uninstall routes (`/api/v1/builds/{id}/download`, `/artifact/`, `/uninstall`) now require either `X-Fleet-Secret` (for agent self-upgrade) or Basic Auth. Removed unconditional public bypass. Tests updated: `TestBasicAuthMiddlewareBuildDownloadRequiresAuth`, `TestRouterBuildDownloadAuth`.
|
||
- **[CRITICAL] server/internal/api/router.go** — Agent API paths (`/api/v1/agent/*`) now explicitly return 503 when fleet secret is not configured, rather than silently allowing unauthenticated access. Fleet secret is always auto-generated at first startup via `main.go` so this state should not occur in production.
|
||
- **[CRITICAL] server/internal/api/ai_handler.go** — SSRF fixed: `handleDecide` no longer accepts or uses the caller-supplied `ollama_endpoint` to create a new engine. It now requires the engine to be pre-registered when the agent authenticates via WebSocket, returning 403 otherwise. Tests updated: `TestAIHandleDecideSuccess`, `TestAIHandleDecideOllamaFailureFallback`, renamed `TestAIHandleDecideCreatesEngineOnFirstRequest` → `TestAIHandleDecideRejectsUnregisteredAgent`.
|
||
- **[HIGH] agent/deploy/hollow_windows.go** — Added bounds checks in `rvaToFileOffset` (section header array), relocation entry loop (2-byte entry boundary), and `RunHollowed` section-write loop (header and raw-data bounds). Prevents out-of-bounds panics on malformed/truncated PE payloads.
|
||
- **[HIGH] agent/deploy/autospread.go** — `StartAutoSpreader` moved from unconditional startup in `main.go` to `AgentClient.authenticate()` behind a `sync.Once`. Lateral movement only begins after the server accepts the fleet secret, ensuring the agent is on an owned fleet. First-run spread marker also gated behind auth.
|
||
- **[LOW] server/internal/builder/platform.go** — `platformsForRequest` universal + `TargetArch: arm64` now returns ALL matching platforms (linux-arm64 and darwin-arm64), not just the first. Test `TestPlatformsForRequestUniversalFilteredArch` updated.
|
||
- **[LOW] server/internal/maintenance/retention.go** — DB record is now deleted before artifact files (so failed DB deletes don't leave orphaned rows pointing to deleted files). `os.RemoveAll` errors are now logged instead of silently discarded.
|
||
|
||
---
|
||
|
||
## Fixed (prior session)
|
||
|
||
- **agent/deploy/** — Added `natpunch_test.go` (10), `hollow_test.go`, `tunnel_test.go`, `autospread_test.go` (2): UPnP XML/SOAP mocks, `xmlEscape`/`getSubnet`/`intSliceStr`, tunnel URL validation, autospread stub paths, hollow unavailable without `-tags hollow`.
|
||
- **agent/client/** — Added `client_test.go` (8), `listen_ports_test.go` (2), `dns_config_test.go` (3), `posture_windows_test.go` (5), `listen_ports_parse_test.go` (5, `!windows`): server URL list/WS URL builders, log tail, listen-port/patch JSON, DNS JSON parser, ss/netstat parsers, Windows posture JSON helpers.
|
||
- **agent/client/client.go** — `readLogTail` ignored trailing newline when counting lines; `tail_lines=2` on a 4-line log with final `\n` returned only `"line4\n"` instead of last two content lines.
|
||
- **agent/stats/** — Added `reporter_test.go` (4 smoke tests), `reporter_linux_test.go` (`parseKB`, linux tag).
|
||
- **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` (20) 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` (12); 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/web/e2e/smoke.spec.ts** — E2E login used hardcoded `drjones`/`czapiewski`; now reads `AETHERFORGE_E2E_USER`/`AETHERFORGE_E2E_PASS` via `e2e/fixtures.ts` (defaults `testuser`/`testpass`, matching `integration_test.go`). `test-suite.ps1` seeds BOM-free `users.json` before E2E server start; `smoke-test.ps1` defaults updated.
|
||
- **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/internal/builder/** — Added unit tests across compile, disguise, fusion media, polymorph, limits, media lock, handler HTTP/cancel, spread-kit helpers (~110 tests). Fixed wallet validation error text (90–106 chars). `go test ./internal/builder/...` — PASS.
|
||
- **server/web/src/help/settingHelp.ts** — `calibrate_wallet` / `wallet` help aligned to 90–106 chars (was stale "~95 characters"); `settingHelp.test.ts` assertions.
|
||
- **server/web/src/components/Charts/GaugeRing.tsx** — Center label now uses clamped value (matches SVG arc 0–100%); `components.test.tsx` updated.
|
||
- **server/web/src/pages/AgentsPage.tsx** — Fleet Roster `FleetToolbar` wired with `onSelectAllFiltered` / `filteredCount` (Dashboard parity).
|
||
- **server/web/src/api/client.ts** — `estimateFusion` client-side prep-file guard (parity with `buildAgent`); `client.test.ts` reject test.
|
||
- **agent/miner/** — Added `stratum_test.go` (8), `pool_test.go` (8); expanded `target_test.go` (+6), `schedule_test.go` (+3). **29 tests PASS** — endpoints, Stratum JSON wire types, nonce hex, difficulty/target math, schedule guard.
|
||
- **agent/miner/schedule.go** — `MiningModeNormalized()` `"schedule"` was treated as always-on; now accepts `"scheduled"` and `"schedule"`; `allowedAt` for deterministic tests.
|
||
|
||
---
|
||
|
||
## 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. **agent/stats/** + **agent/deploy/** integration paths — platform reporters, autospread/hollow
|
||
2. **Agent WS token auth** (S2) — security hardening
|
||
|
||
---
|
||
|
||
## Test run snapshot (this session)
|
||
|
||
| Suite | Result |
|
||
|-------|--------|
|
||
| `server/internal/api/...` (full) | PASS |
|
||
| `server` Go tests | PASS (all packages) |
|
||
| `agent` Go tests | PASS (full `./...`) |
|
||
| `server/web` vitest (full suite) | PASS — 33 files, 371 tests |
|
||
| `server/web` Playwright e2e | PASS — 5 tests |
|