randomayzer/agents/codex/done/TASK-2026-10-19-coder2-review.md

61 lines
4.5 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Result — coder2 review and scoped fixes
Base SHA: `6382cb8e0473ba9ea5ba7a3f94f34e8ab0d4ec60`.
Reviewed/result SHA: `6382cb8e0473ba9ea5ba7a3f94f34e8ab0d4ec60` (no commit; shared working tree remains uncommitted).
## Scope reviewed
Reviewed the full stage-one working diff and added deployment/R3 files against the stage-one manifest and R1–R5 assignments. Focused on production ingress trust, auth profiles, Compose isolation, backup/restore and migration bootstrap, VK incomplete-data handling, the A–H smoke script, and runbook correctness. Preserved the existing local `AGENTS.md` modification and the owner-authorized four seed-length assertion changes. No production host, firewall, WireGuard, tunnel, or Docker state was changed by this review.
## Fixes made
- Restored the original manual smoke follow-ups I–M for snapshot, draw, public audit, refresh, and logout. The script continues to report only its A–H automation and never claims these follow-ups passed.
- Changed the local smoke example from `127.0.0.1` to canonical `localhost`, matching the production profile and callback origin.
- Documented the production entrypoint's socket-peer sanitization and the distinction from generic `TRUST_PROXY=true` behavior in `docs/PRODUCTION_GUARDS.md`.
- Made the smoke script stop with a `FAIL setup` result if `mktemp` cannot create its private directory, avoiding accidental writes to root paths.
- Returned the internally generated incomplete-data reason to the UI instead of replacing it with a generic message; added a route regression assertion and confirmed the prior participant list remains unchanged on failure.
- Changed backup-directory setup in the runbook to create it with the operator's ownership and mode `0700`.
## Files changed by this review
- `agents/codex/inbox/TASK-2026-10-19-coder2-review.md`
- `docs/PRODUCTION_GUARDS.md`
- `docs/VK_REAL_SMOKE_TEST.md`
- `docs/STAGE1_DEPLOYMENT_RUNBOOK.md`
- `tools/vk-smoke.sh`
- `src/core/errors/http-errors.ts`
- `tests/vk-r3-safety.test.ts`
- `agents/codex/done/TASK-2026-10-19-coder2-review.md`
## Commands actually run
- `USE_VK_MOCK=true npx vitest run tests/vk-r3-safety.test.ts tests/deployment-profile.test.ts tests/production-transport.test.ts` — exit 0; 3 files, 26 tests passed.
- `bash -n tools/vk-smoke.sh tools/db-backup.sh tools/db-backup-daily.sh tools/db-restore-check.sh` — exit 0.
- `git diff --check` — exit 0.
- `git rev-parse HEAD` — `6382cb8e0473ba9ea5ba7a3f94f34e8ab0d4ec60`.
The full test/lint/build/audit gates and local Docker/restore acceptance were not rerun by this reviewer; coordinator owns the final shared gates and Docker smoke. No live VK account, production host, or VPS was used.
## Findings
- CRITICAL: none found in reviewed scope.
- HIGH: none found in reviewed scope.
- MEDIUM: none found in reviewed scope.
- LOW: fixed in the working diff — smoke temporary-directory creation lacked a failure guard; previous checklist I–M had been dropped from the updated runbook; local smoke example used a host different from the configured canonical origin; incomplete-data UI response did not expose its safe cause; backup directory creation did not establish the service account's ownership.
These are coder2 review results, not an independent security sign-off. Auth, proxy trust, migrations, and the production Compose boundary still require the requested independent audits.
## UNVERIFIED
- Live VK ID redirect acceptance for localhost, consent, token refresh, and real API behavior/scopes/limits.
- Whether Docker ingress on the target host preserves the actual VPS socket peer `10.77.0.1`; the transport tests cover forged headers and an actual loopback socket, not the VPS path.
- VPS identity, existing web server, DNS, certificate, firewall persistence, domain login, and network isolation acceptance.
- The actual Caddy version/directive compatibility on the operator's VPS; deployment snippets must be validated against the installed server.
## Remaining blockers
Live VK credentials/consent and operator confirmation/actions for R5 remain external prerequisites. Coordinator-reported local Docker backup-restore success is not independently reproduced in this review. The phase remains open for the two independent auditors and coordinator's final gates.
## Non-blocking tech debt / environment notes
`MEMORY_POLICY.md` is at `/srv/projects/AI-Memory/00_SYSTEM/MEMORY_POLICY.md` alongside `AGENT_PROTOCOL.md`; it was read. Per coordinator direction, only an individual worklog is being added; shared project-state files are left untouched.