diff options
Diffstat (limited to 'FIXES.md')
| -rw-r--r-- | FIXES.md | 78 |
1 files changed, 78 insertions, 0 deletions
diff --git a/FIXES.md b/FIXES.md new file mode 100644 index 0000000..e5d5fc6 --- /dev/null +++ b/FIXES.md @@ -0,0 +1,78 @@ +# Fix Tranche 1 — verified P0/P1 plus high-value P2s + +Date: 2026-10-05 +Companion to `AUDIT.md` (findings) and `reports/audit/` (raw lane reports). + +## Status + +- `go build ./...` — pass +- `go vet ./...` — pass +- `make lint` (`golangci-lint run`) — **now 0 issues** (was 15) +- `go test ./...` — pass +- `go test -race` (engine, net, persistence, player, world, combat, casino, game, hacking) — pass +- Changes are uncommitted. `git diff --shortstat` = 51 files, +509/−131 (plus new test files). + +Nothing was staged or committed. The audit artifacts (`AUDIT.md`, `reports/`) are locally excluded via `.git/info/exclude` so they do not dirty the tree. + +## Fixed (each with a regression test unless noted) + +### Correctness / exploit (P0/P1) +1. **Gather verbs broken (P0)** — restored `mine/chop/fish/cut → gather` aliases in `internal/game/act.go`; corrected the misleading comment. Test: `TestGatherVerbsNormalizeToGatherAction`. +2. **`walk <huge>n` remote OOM** — `parseWalkDirections` now clamps the count before allocating (`internal/game/cmd_walk.go`). Test: `TestParseWalkDirectionsRejectsHugeCount`. +3. **`sell`/`deposit` dead-end on 2+ stacks** — dedup by name with `uniqueItemNames` (`cmd_shop.go`, `cmd_bank.go`). +4. **Admin `//` path bypass** — added `canonicalDataRelPath`, classify on the canonical path, removed the shadowed `fullPath` join (`internal/admin/api_files.go`); hardened the game-side `playerDataFileName` the same way. Tests: `TestCanonicalDataRelPathStripsRepeatedPrefixes`, `TestPlayerDataFileNameCanonicalizesPrefixes`. +5. **Undo can truncate a live file to 0 bytes** — `load()` maps an empty before-image back to nil so `requireSnapshot` fires (`internal/admin/undo.go`). (Fail-safe: an intentionally empty before-image is refused rather than written; the explicit-existence-flag fix remains a follow-up.) +6. **Roulette selective-refund exploit** — disconnect during `phasePlaying` now forfeits the stake, matching Big 6 (`internal/casino/roulette.go`); updated `TestRouletteRefundAndPersistenceFailure` to assert forfeit vs refund. +7. **Resolved predictions destroyed by the credit cap** — compute totals and run the cap guard before mutating contract status (`internal/game/predictions.go`). Test: `TestCollectPredictionsKeepsContractsWhenCreditCapBlocks`. +8. **`RekeyRooms` stranded wanderers** — rebuild the object-state map from every state and remap `RoomID`/`HomeRoomID`/`WanderRooms` (`internal/world/world.go`). Test: `TestRekeyRoomsUpdatesAwayFromHomeObject`. +9. **Admin room swap/undo did not reconcile live state** — `handleRoomSwap` now reconciles both rooms; `rebuildAfterUndo` reconciles when the room file still exists (`internal/admin/api_rooms.go`, `server.go`). +10. **Estate directory was dead** — allow `estate_directory` through the interactability gate (`act.go`); exempt the `Cfg == nil && EstateDirectory` prompt in `advanceTalk` (`act_talk.go`). Live content places it in `data/rooms/tuccia/3124.yaml`. +11. **`ui.Table` title overhang** — truncate the title to the interior width (`internal/ui/table.go`). Test: `TestTableLongTitleStaysWithinBorder`. +12. **Hacking minigames printed raw color tags** — expand tags before writing (`internal/game/core_hacking.go`). +13. **Pai gow betting window never expired with one bettor + one idle seat** — count seated players, not bettors (`internal/casino/pai_gow.go`). Test: `TestPaiGowBettingWindowExpiresWithOneBettorAndIdleSeat`. + +### Combat / player / items +14. **Double combat entry** — `startCombat` now refuses re-entry (`internal/game/combat_attack.go`); `Tracker.Enter` frees the previous mob (`internal/combat/state.go`). Test: `TestTrackerEnterReleasesPreviousMob`. +15. **Retribution skipped `StartRegen`** — surviving mob no longer heals 100× too fast (`internal/game/combat_mob.go`). +16. **`RemoveItem` drained items on failure** — now all-or-nothing (`internal/player/player.go`). Test: `TestRemoveItemIsAllOrNothing`. +17. **`drop all <item>` zeroed quality** — preserve Quality/MaxQuality via `AddGroundItemQ` (`internal/game/cmd_drop.go`). +18. **`RegenerateTick` was persisted** — added `yaml:"-"` (`internal/player/player.go`). +19. **`XPTable` was `[100]int` with 99 values** — now `[99]int` (`internal/player/xp.go`). Test: `TestXPTableBounds`. + +### Command / UX +20. **Prefix/abbreviation lowercased arguments** — preserve original case for `rawInput` consumers (`internal/game/game.go`). Test: `TestGlobalCommandPreservesArgumentCase`. +21. **`queued` ignored the consume slot** — added `PeekConsume` and display (`core_command_queue.go`, `cmd_queued.go`). +22. **`queued` hardcoded 600 ms** — now reports the configured tick length (`cmd_queued.go`). +23. **`verbs` ignored `wrap_width`** — honors the option (`cmd_verbs.go`). +24. **Unknown craft subtype soft-locked the action** — fallback uses `behavior.TypeCraft` so it advances (`production_engine.go`). +25. **Empty-ingredient craft was infinite/free** — `planIngredients` returns nil (`production_index.go`). Test: `TestPlanIngredientsRejectsEmptyCraft`. + +### Persistence / validation / net +26. **`Move` + `Delete`/`Replace` silently lost files** — conflicting ops now error (`internal/persistence/txn.go`). Tests: `TestTxnRejectsConflictingMoveThenDelete`, `TestTxnRejectsConflictingMoveThenReplace`, `TestTxnMoveThenReplaceDestination`. +27. **`Recover` dropped unknown op kinds and deleted the journal** — unknown kind is now a rollback error, keeping the journal (`txn.go`). Test: `TestRecoverKeepsJournalWithUnknownOpKind`. +28. **Journal directory entry not durable** — `Recover` (the startup path that creates it) now fsyncs its parent (`txn.go`). +29. **`ExitDef.Condition` never validated** — added the check (`internal/validate/checks.go`). +30. **`stripIAC` leaked a byte for a truncated IAC** — drop the remainder (`internal/net/conn.go`). Test: `TestStripIACDropsTruncatedSequence`. + +### Tooling +31. **`make lint` gate restored** — fixed the 15 reported issues (error-string casing, doc-comment form, unused params, ineffassign) without weakening `.golangci.yml`. + +## Deliberately deferred (still open) + +These are documented in `AUDIT.md`; the next tranche should cover them. + +- **Undo failure reporting** (`Undo()`/`Redo()` still return nil on failure → "Nothing to undo"). Needs a signature change. +- **Explicit snapshot-existence flag** to allow undoing an intentionally empty file (current fix is a safe refusal). +- **`quit` queue/busy interaction**; **bank withdraw duplicate ambiguity list**; **`get <n> credits` ignores `<n>`**. +- **Production `FirstCraft()` vs multi-recipe availability**; **"how many" batches vs units**. +- **Room object reference entries silently drop behavior fields**; **`.yml` files invisible to validation**; **craft `type`/`subtype` validator rule**; **talk `start` node check**; **safespot validation**; **drop weight validation**. +- **Persistence `Recover` mid-game misuse**, journal versioning, `.save-*` temp sweep. +- **Casino**: craps Fire/ATS deferral, shooter-change reset, video-poker raw error text, slots free-spin off-by-one, plinko RTP wording, machine leave silently dropping a banked feature, cabinet ID collisions. +- **Admin**: `restoreAccountSecret` fail-open, `entityCreate` persisting `_path`/`_raw`, trigger POST/PUT divergence, `validEntityID`, `resolve-duplicate`, course detach, symlink handling. +- **Net**: telnet echo CRLF deadline, shutdown flush starvation, `armReadDeadline` concrete-type switch, hub bucket pruning. +- **Structural refactors** from `AUDIT.md` §3–4 (Table shrink consolidation, `countExtraInbound`, cert gen, YAML encoders, `tableCore` for pai gow, typed `GridConflict.Kind`, etc.). +- **Test infrastructure**: `make test-race`/CI, harness owner-goroutine leak, master-tick harness option, and coverage for steal/gather/burn/talk and the WebSocket transport. + +## Suggested next step + +Continue with a second tranche focused on the remaining P1/P2 correctness items and the admin/persistence/validation gaps, then a structural-refactor tranche. Each change should keep `make lint`, `go test ./...`, and `go test -race` green. |