From 148e6c1a8308627917de6aec116deceb6ff79060 Mon Sep 17 00:00:00 2001 From: lorentz Date: Fri, 10 Jul 2026 18:21:50 -0400 Subject: [PATCH] docs(phase-10): add/update security threat verification --- .../10-SECURITY.md | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 .planning/phases/10-pax8-client-auth-foundation/10-SECURITY.md diff --git a/.planning/phases/10-pax8-client-auth-foundation/10-SECURITY.md b/.planning/phases/10-pax8-client-auth-foundation/10-SECURITY.md new file mode 100644 index 0000000..5c15842 --- /dev/null +++ b/.planning/phases/10-pax8-client-auth-foundation/10-SECURITY.md @@ -0,0 +1,56 @@ +--- +phase: 10 +slug: pax8-client-auth-foundation +status: verified +threats_open: 0 +asvs_level: 1 +created: 2026-07-10 +--- + +# Phase 10 — PAX8 Client & Auth Foundation — Security Audit + +**Audited:** 2026-07-10 +**ASVS Level:** 1 +**Block on:** high +**Threats:** 11/11 CLOSED, 0 OPEN + +## Scope + +Verified against implementation, not documentation or intent: +- `lib/types/pax8.ts`, `lib/services/pax8-client.ts`, `lib/services/pax8-factory.ts` +- `lib/services/pax8-client.test.ts`, `lib/services/pax8-factory.test.ts` (executed: 12/12 pass) +- `migrations/091_pax8_tables.sql` +- `scripts/verify-pax8-auth.ts` +- `.gitignore`, `CLAUDE.md` + +No SUMMARY.md in this phase (10-01, 10-02, 10-03) contains a `## Threat Flags` section — no executor-flagged new attack surface to reconcile. A repo-wide grep confirmed no `app/api/**` route yet imports `pax8-client`/`pax8-factory`, consistent with the phase's own trust-boundary claim of "no inbound user-facing route" — no unregistered attack surface found. + +## Threat Verification + +| Threat ID | Category | Disposition | Verification | Evidence | +|-----------|----------|-------------|---------------|----------| +| T-10-01 | Information Disclosure | mitigate | grep: `clientSecret` in pax8-client.ts appears only in `Pax8ClientConfig` type and inside `JSON.stringify({..., client_secret: this.config.clientSecret})`; both throws (`getToken`, `fetchJson`) interpolate only `res.status`/response `text`, never `config.clientSecret`. Same in pax8-factory.ts: `CLIENT_SECRET` used only in `process.env` reads, never in the thrown `Error(...)` or `console.log`. | `lib/services/pax8-client.ts:36,43,69`; `lib/services/pax8-factory.ts:6,12,16,18` | +| T-10-02 | Information Disclosure | accept | `accessToken` is a private class field; grep confirms zero `console.*(accessToken` / log statements anywhere referencing it; no persistence call (no `postgresClient`/file write) touches it. Server-side singleton only (`pax8-factory.ts` module-level `_client`). Accepted-risk entry recorded below. | `lib/services/pax8-client.ts:16,24-25,47,49` | +| T-10-03 | Spoofing/Tampering (MITM) | mitigate | Both `fetch()` calls hardcode `https://api.pax8.com/...`; no `process.env` host override, no `http://` fallback anywhere in the file. | `lib/services/pax8-client.ts:30,57` | +| T-10-04 | Elevation of Privilege | mitigate | `audience: 'https://api.pax8.com'` is a hardcoded string literal in the token POST body, not sourced from env/config. | `lib/services/pax8-client.ts:37` | +| T-10-05 | Denial of Service (self-inflicted) | mitigate | Cache check is exactly `Date.now() < this.tokenExpiry - 60000`, gating reuse with a 60s buffer before presenting a near-expired token. | `lib/services/pax8-client.ts:24` | +| T-10-06 | Tampering (SQL injection) | accept | `migrations/091_pax8_tables.sql` contains zero `INSERT`/dynamic SQL; it is 100% static `CREATE TABLE`/`CREATE INDEX`/`COMMENT ON` DDL with no interpolated values. Accepted-risk entry recorded below. | `migrations/091_pax8_tables.sql` (full file) | +| T-10-07 | Denial of Service | mitigate | All 6 `CREATE TABLE IF NOT EXISTS`, all indexes `CREATE INDEX IF NOT EXISTS` / `CREATE UNIQUE INDEX IF NOT EXISTS`; no `DROP`/`ALTER` statement anywhere in the file; 10-02-SUMMARY.md records a live second-apply against `pulse-postgres` returning `NOTICE: already exists, skipping` with exit 0. | `migrations/091_pax8_tables.sql:28,54,75,99,123,147,159-167` | +| T-10-08 | Information Disclosure | accept | `raw_payload JSONB` columns declared on all 5 primary/child tables; migration contains no `INSERT` — columns are schema-only and empty in this phase. Accepted-risk entry recorded below. | `migrations/091_pax8_tables.sql:38,60,83,106,131` | +| T-10-09 | Information Disclosure | mitigate | `.gitignore:34` is `.env*`; `git ls-files \| grep -i '\.env'` returns no tracked env file. Verify script's own header comment directs credentials to `.env.local`. | `.gitignore:34`; `scripts/verify-pax8-auth.ts:9-10` | +| T-10-10 | Information Disclosure | mitigate | `scripts/verify-pax8-auth.ts` logs only `result.content.length` and `result.page.totalElements`; the catch block logs `err.message` — traced back through T-10-01, the underlying error strings never contain the secret. grep for `access_token`/`clientSecret`/`CLIENT_SECRET` value-logging returns none (the one hit is a doc comment naming the env var, not its value). | `scripts/verify-pax8-auth.ts:26-38` | +| T-10-11 | Elevation of Privilege | mitigate | 10-03-SUMMARY.md records the developer-run live proof: token exchange succeeded and `/companies` returned data (118 total companies) — no 403, confirming the hardcoded `audience` is correctly scoped, exercised against the real API rather than only asserted in code. | `.planning/phases/10-pax8-client-auth-foundation/10-03-SUMMARY.md:47,60,109-117` | + +## Accepted Risks Log + +- **T-10-02** (in-memory token cache): Accepted. Token lives only in a private `Pax8Client.accessToken` field, process-local, never persisted to disk/DB/Redis, never logged. Matches the established `msgraph-client.ts` pattern already accepted elsewhere in this codebase. No compensating control needed beyond the existing server-side-only execution boundary. +- **T-10-06** (static migration DDL): Accepted. `migrations/091_pax8_tables.sql` has no dynamic/interpolated SQL and no request/user data flows into it — SQL injection is not a reachable vector for this file. +- **T-10-08** (`raw_payload JSONB` columns): Accepted for this phase only. Columns are empty (schema-only migration, no sync logic yet). Once Phase 11+ populates them, they will hold PAX8 business data inside the same trusted Postgres instance already holding Autotask data — no new exposure surface is introduced by this phase. **Follow-up:** Phase 11/12's security audit must re-verify this acceptance once `raw_payload` is actually populated (e.g., confirm no PII/secret fields from PAX8 payloads land in a column exposed by an unauthenticated route). + +## Unregistered Flags + +None. No SUMMARY.md in this phase declares a `## Threat Flags` section, and a targeted grep of `app/api/**` confirms no route yet imports `pax8-client`/`pax8-factory` — the phase introduces no new inbound attack surface beyond what the trust-boundary table already declares (outbound-only: Pulse -> PAX8 API). + +## Notes for Next Phase + +Phase 11 (current-state sync) and Phase 12 (historical sync + company matching) will introduce the first read paths into `raw_payload` and the first write paths from real PAX8 data. Re-run threat verification against T-10-08's follow-up note, and register any new inbound routes (e.g., admin UI reads of `pax8_company_match_review`) with their own STRIDE entries before they ship.