Files
familysync/.planning/phases/18-auto-timezone-detection-and-ability-to-change-timezone/18-SECURITY.md
T
Lucas BergerandClaude Opus 4.8 745e806d89 docs(phase-18): add security threat verification (SECURED, 0 open)
gsd-security-auditor verified all 13 plan-time STRIDE threats against the
implementation: 8 mitigate confirmed in code (file:line evidence), 5 accept
documented, 4 supply-chain checks (zero new deps). threats_open: 0. ASVS L1,
block_on high — no high-severity gaps. Post-review fixes (WR-01 blank-TZ guard,
WR-02 INSERT IGNORE) verified in code; D-07 boundary confirmed via git diff.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-15 07:58:53 -04:00

72 lines
6.3 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.
---
phase: 18-auto-timezone-detection-and-ability-to-change-timezone
audited: 2026-06-15
status: secured
asvs_level: 1
block_on: high
register_authored_at_plan_time: true
threats_total: 13
threats_closed: 13
threats_open: 0
threats_accepted: 4
supply_chain_checks: 4
---
# Phase 18 — Auto Timezone Detection & Change Timezone: Security Audit
**Audited:** 2026-06-15
**ASVS Level:** 1
**block_on:** high
**Compared against:** origin/main..HEAD
**Status:** SECURED — 13/13 threats closed (8 mitigate verified, 5 accept documented), 4× T-18-SC supply-chain verified
This audit verifies each declared threat mitigation EXISTS in the implemented code. It does not scan for new vulnerabilities. Implementation files were not modified.
## Threat Verification
| Threat ID | Category | Disposition | Status | Evidence |
|-----------|----------|-------------|--------|----------|
| T-18-01 | Tampering | mitigate | CLOSED | `householdTimezone.ts:25-35` reads only; never returns an unvalidated forwarded write. `resolveHouseholdTimezone` (`:49-61`) guarantees non-empty fallback (trim guard). All writes go through Plan 02 IANA-validated path. |
| T-18-02 | DoS | accept | CLOSED | Accepted risk logged below. Single PK lookup per scheduler tick (`reminderScheduler.ts:250`). |
| T-18-03 | Elevation of Privilege | mitigate | CLOSED | `admin.ts:42` `adminRouter.use('*', requireAdmin)` is the FIRST router statement, before all routes. GET (`:200`), PUT (`:224`), POST seed (`:251`) all appended after it → inherit the guard. 403 tests exist (`admin.test.ts:603-625, 711-720`). |
| T-18-04 | Tampering | mitigate | CLOSED | `admin.ts:56-62` `timezoneSchema` uses `.refine(isValidIanaTimezone)`; PUT (`:224`) and seed (`:251`) both bind it via `zValidator('json', timezoneSchema)`. `isValidIanaTimezone` (`householdTimezone.ts:70-77`) is eval-free try/catch on `Intl.DateTimeFormat`. Invalid → 400 before any DB write. |
| T-18-05 | Tampering | mitigate | CLOSED | Seed (`admin.ts:262-266`) uses `INSERT IGNORE`; an existing row is silently ignored (value preserved, D-03 no-overwrite). `seeded` derived from `affectedRows === 1`. Cannot overwrite an existing value. |
| T-18-06 | Information Disclosure | accept | CLOSED | Accepted risk logged below. Grep confirms no `console.log` of request bodies in `admin.ts` (only a T-10-10 comment reference). Timezone is non-sensitive. |
| T-18-07 | Injection | mitigate | CLOSED | PUT upsert (`admin.ts:227-230`) uses Drizzle `.insert().onDuplicateKeyUpdate` — parameterized, key is hard-coded literal `'household_timezone'`. Seed (`:262-263`) uses a `sql` template where `INSERT IGNORE` is a literal keyword, column identifiers via `sql.identifier`, and `${timezone}` is a bound parameter (not string-concatenated) and IANA-validated upstream. |
| T-18-08 | Tampering | mitigate | CLOSED | Broker sites consume `getHouseholdTimezone(db)` only (`reminderScheduler.ts:250`, `outboxWorker.ts:391`). `resolveHouseholdTimezone` (`householdTimezone.ts:49-61`) trims stored value and `process.env.TZ`; empty/blank falls through to a valid Intl zone — fallback can never return `''`/invalid. |
| T-18-09 | Tampering (regression) | mitigate | CLOSED | `git diff --name-only origin/main..HEAD` excludes `eventDateTime.ts` and `hydrateEvents.ts` (D-07 boundary intact). No timed-write/display path touched. |
| T-18-10 | DoS | accept | CLOSED | Accepted risk logged below. Per-drain-cycle memoization (`outboxWorker.ts:385-395`, created `:774`) shares one app_config read across both all-day branches (`:527`, `:634`). |
| T-18-11 | Elevation of Privilege | accept | CLOSED | Accepted risk logged below. Client gate is UX-only; server `requireAdmin` (`admin.ts:42`) is the real control. Client (`client.ts:498`) documents server enforcement. |
| T-18-12 | Tampering | mitigate | CLOSED | Client `setAdminTimezone` (`client.ts:501-511`) sends raw input to server; server `timezoneSchema.refine` (`admin.ts:56-62`) is authoritative (400 on invalid). Free-text input (`AdminPage.tsx:387-406`) is not the security boundary. |
| T-18-13 | Information Disclosure | mitigate | CLOSED | Timezone rendered as plain-text JSX (`AdminPage.tsx:431` `Use detected: {detectedTz}`) and as controlled input `value={effectiveTimezoneInput}` (`:390`). Grep confirms NO `dangerouslySetInnerHTML` in AdminPage.tsx. |
| T-18-SC (×4, plans 01-04) | Supply chain | mitigate | CLOSED | `git diff origin/main..HEAD` against all `package.json` / `pnpm-lock.yaml` returns EMPTY — zero new dependencies. PWA picker uses built-in `Intl`. |
## Accepted Risks Log
- **T-18-02 (DoS — DB read per scheduler tick):** Single primary-key lookup on `app_config` per 60s scheduler interval. Negligible load; read-per-run chosen so timezone changes propagate within one tick without a worker restart. Accepted.
- **T-18-06 (Information Disclosure — log/echo of submitted timezone):** Timezone identifiers are non-sensitive (not credentials or PII). No `noEchoHook` required; verified no body logging in handlers. Accepted.
- **T-18-10 (DoS — extra DB read at 3 call sites):** Mitigated in practice by per-drain-cycle memoization; PK lookups on a 60s interval are negligible. Accepted.
- **T-18-11 (Elevation of Privilege — client renders admin UI from isAdmin flag):** The client `isAdmin` gate is a UX convenience only. A forged request still hits server-side `requireAdmin` → 403 (T-18-03). The UI gate is not relied upon as a security control. Accepted.
## Unregistered Flags
None. All four plan SUMMARY `## Threat Flags` sections declare "No new threat surface beyond the plan's threat model." No new endpoints, auth paths, file-access patterns, or schema changes appeared during implementation that lack a mapped threat ID.
## Notes
- Post-review fixes (18-REVIEW-FIX.md: WR-01, WR-02, IN-01/02/03) were verified in code, not accepted on documentation alone:
- WR-01 empty/blank TZ guard present at `householdTimezone.ts:50-58` (relevant to T-18-08).
- WR-02 `INSERT IGNORE` + affectedRows-derived `seeded` present at `admin.ts:262-266` (relevant to T-18-05).
- D-07 boundary independently confirmed via `git diff --name-only`.
- Zero-dependency claim independently confirmed via empty manifest/lockfile diff.
## Security Audit 2026-06-15
| Metric | Count |
|--------|-------|
| Threats found | 13 |
| Closed | 13 |
| Open | 0 |
| Accepted risks | 4 |
| Supply-chain checks | 4 |