docs(phase-10): add/update security threat verification
This commit is contained in:
parent
253e37e1e2
commit
148e6c1a83
1 changed files with 56 additions and 0 deletions
|
|
@ -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.
|
||||
Loading…
Add table
Add a link
Reference in a new issue