62 lines
6.9 KiB
Markdown
62 lines
6.9 KiB
Markdown
# Result — auditor1 independent review
|
||
|
||
Base SHA: `6382cb8e0473ba9ea5ba7a3f94f34e8ab0d4ec60`
|
||
Reviewed SHA: `17faecd962c2cf40e0a3f6ec4434811cfffdd312`
|
||
Result SHA: `17faecd962c2cf40e0a3f6ec4434811cfffdd312` (review only; no code change)
|
||
Working-tree context: pre-existing `AGENTS.md` modification was preserved and excluded from the reviewed commit range.
|
||
|
||
Scope: R1–R5 acceptance review with focus on production auth profile and CSRF/Secure cookie strictness, socket-derived proxy trust, deployment networks/backups, R3 fail-closed/no-partial-save behavior, and smoke-report truthfulness.
|
||
|
||
## Finding
|
||
|
||
### MEDIUM — Live VK smoke can report mock-provider results as real VK checks
|
||
|
||
Files/lines: [`tools/vk-smoke.sh`](/srv/projects/Agent%20projects/randomayzer/tools/vk-smoke.sh:105), [`src/providers/factory.ts`](/srv/projects/Agent%20projects/randomayzer/src/providers/factory.ts:8), [`src/providers/vk/vk-mock-provider.ts`](/srv/projects/Agent%20projects/randomayzer/src/providers/vk/vk-mock-provider.ts:113).
|
||
|
||
`ProviderFactory.getVkProvider()` selects `VkMockProvider` whenever `USE_VK_MOCK=true`, before it checks for `VK_SERVICE_TOKEN`. The production Compose service inherits arbitrary values from its configured `env_file` and does not reject or override this flag. The smoke script never confirms which provider answered: D accepts a mock `resolvedAuthType: SERVICE` as `PUBLIC_SERVICE`; F/G/H can create a database record, import generated participants, and receive generated subscription statuses. E may be omitted (`VK_SMOKE_PRIVATE_POST_URL` is optional). The script exits successfully whenever there are no `FAIL` lines; `SKIP` does not make the run fail (lines 176–177).
|
||
|
||
Reproducer: configure the production env file with `USE_VK_MOCK=true` alongside the normal credentials, complete the real browser consent for A–C, set a public post URL and `VK_SMOKE_CREATE_GIVEAWAY=YES`, and omit `VK_SMOKE_PRIVATE_POST_URL`. The application serves the mock provider despite a service token; A–C can pass from the actual auth flow, D and F/G/H can report `PASS` from generated data, E reports `SKIP`, and the script exits 0. This does not demonstrate public post retrieval, participant import, or subscription checks against VK. It makes the smoke result insufficient evidence for those live checks.
|
||
|
||
## Other severity levels
|
||
|
||
CRITICAL: none found in the reviewed scope.
|
||
|
||
HIGH: none found in the reviewed scope.
|
||
|
||
LOW: none found in the reviewed scope.
|
||
|
||
## Review notes
|
||
|
||
- Production auth configuration uses canonical `APP_BASE_URL`, permits HTTP only when `DEPLOYMENT_PROFILE=local-single-user` and the origin is loopback, rejects local-profile HTTPS/LAN/public origins, requires matching callback origin/path, and makes the session cookie Secure in strict production. CSRF uses configured origin in production, checks scheme as well as host, ignores request Host and forwarded Host, and requires Origin/Referer for production mutations.
|
||
- The supplied production entry point removes client proxy identity headers and derives trust from `req.socket.remoteAddress`; only `10.77.0.1` may supply a single valid forwarded IP and HTTPS scheme. It overwrites Host/forwarded host from the canonical origin. The Compose files map the app to loopback and (in the domain overlay) the WG address, leave PostgreSQL unpublished, and declare dedicated named resources and CPU/memory limits. Actual Docker/NAT socket peer behavior and host firewall rules remain operator-side acceptance checks.
|
||
- R3 pagination checks totals, malformed/oversized/short pages and page ceilings. Subscription enrichment rejects absent, duplicate, unexpected, or malformed statuses. The participant import route only calls the atomic store update after fetch and enrichment complete, and its focused test proves an incomplete subscription response preserves prior participants. Unsupported repost/admin rules fail with explanatory capability errors. The code uses ordinary VK methods and does not call VK `execute`.
|
||
- Backup commands use a 0600 env-file check and restrictive umask; the daily workflow validates archive readability, restores to a new temporary database with `--exit-on-error`, and applies retention only after the restore check succeeds. Actual backup/restore and daily timer execution are not evidenced by this review.
|
||
- The migration bootstrap was inspected for scope and the runbook guards existing databases against applying the initial CREATE schema over existing tables. Root owns the clean PostgreSQL migration/integration gate.
|
||
|
||
## Commands actually run
|
||
|
||
- `git status --short` — showed only pre-existing ` M AGENTS.md` before this review.
|
||
- `git rev-parse HEAD` and `git rev-parse <base>^{commit}` / `git rev-parse <reviewed>^{commit}` — HEAD and reviewed SHA are `17faecd962c2cf40e0a3f6ec4434811cfffdd312`; base resolved to `6382cb8e0473ba9ea5ba7a3f94f34e8ab0d4ec60`.
|
||
- `git diff --stat` and `git diff --name-status <base>..<reviewed>` — inspected the full 51-file change set.
|
||
- `npx vitest run tests/deployment-profile.test.ts tests/production-transport.test.ts tests/vk-r3-safety.test.ts` — exit 0; 3 files and 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.
|
||
- `node --check tools/production-transport.cjs` — exit 0.
|
||
- `node --check tools/serve-production.cjs` — exit 0.
|
||
- `git diff --check <base>..<reviewed>` — exit 0.
|
||
- Full `npm ci`, Prisma generate, full tests, lint, build, npm audit, real PostgreSQL migration/integration and Docker stack/backup-recovery gates were not run by this auditor; root owns them.
|
||
|
||
## UNVERIFIED
|
||
|
||
- Real VK ID browser consent and the actual support of the loopback redirect URI.
|
||
- Live VK behavior for public/restricted posts, pagination, subscription responses and scopes; the smoke false-positive finding above must be addressed before D/F/G/H output can serve as live evidence.
|
||
- R5 DNS, VPS identity, existing proxy server/config, certificate issuance, Basic Auth 401, live OAuth over the domain, Docker-preserved socket peer address, host firewall persistence, and internet/LAN/WG isolation.
|
||
- Actual daily backup, restore drill, retention, service timer and production rollback.
|
||
- Full R1 audit and R2 clean real-PostgreSQL acceptance are outside this review's executed checks.
|
||
|
||
## Remaining blockers and non-blocking tech debt
|
||
|
||
- Blocker for treating R3 smoke D/F/G/H as live acceptance: prevent `USE_VK_MOCK=true` in production (or make the smoke prove that real-provider mode is active) and rerun against VK. Keep the result for this finding open until the false-positive path is removed and independently rechecked.
|
||
- R5 and R4 operator acceptance remain pending per the runbook; this review does not mark any phase closed.
|
||
- No additional non-blocking tech debt identified in the reviewed acceptance scope.
|
||
|
||
Final verdict: one MEDIUM finding; do not treat live VK smoke checks as accepted evidence until fixed and rerun. No CRITICAL/HIGH finding was established by this review.
|