Expand test coverage across server, agent, and web; fix bugs found during audit.

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.
This commit is contained in:
AetherForge
2026-05-31 01:13:49 -07:00
parent 159747877c
commit ea6f54ad03
89 changed files with 5307 additions and 322 deletions

View File

@@ -10,60 +10,68 @@ Findings from systematic bug-hunt and test expansion (May 2026).
### 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/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/**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 0100%; 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 90106 (same drift fixed in forgeCompatibility / cheatSheetContent troubleshoot).
- [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.)
---
## Fixed (this session)
## 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 165535, 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.
@@ -80,7 +88,7 @@ Findings from systematic bug-hunt and test expansion (May 2026).
- **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/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 90106 chars. Exported `formatBytes` helper.
- **server/web/src/pages/SettingsPage.tsx** — Wallet placeholder aligned to 90106 chars. Exported `deepMerge` helper (config import).
@@ -94,9 +102,9 @@ Findings from systematic bug-hunt and test expansion (May 2026).
- **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/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/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/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.
@@ -108,7 +116,13 @@ Findings from systematic bug-hunt and test expansion (May 2026).
- **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 90106 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.
- **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 (90106 chars). `go test ./internal/builder/...` — PASS.
- **server/web/src/help/settingHelp.ts** — `calibrate_wallet` / `wallet` help aligned to 90106 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 0100%); `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.
---
@@ -120,10 +134,8 @@ See git history and prior audit IDs (B1B42, C1C6, H1H8, etc.) in README
## Recommended next section
1. **server/web/src/help/settingHelp.ts** — align wallet help text to 90106 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
1. **agent/stats/** + **agent/deploy/** integration paths — platform reporters, autospread/hollow
2. **Agent WS token auth** (S2) — security hardening
---
@@ -131,11 +143,8 @@ See git history and prior audit IDs (B1B42, C1C6, H1H8, etc.) in README
| 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 |
| `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 |