diff --git a/.planning/phases/16-eml-mime-evidence-parser/16-02-SUMMARY.md b/.planning/phases/16-eml-mime-evidence-parser/16-02-SUMMARY.md new file mode 100644 index 0000000..f5ba5a8 --- /dev/null +++ b/.planning/phases/16-eml-mime-evidence-parser/16-02-SUMMARY.md @@ -0,0 +1,122 @@ +--- +phase: 16-eml-mime-evidence-parser +plan: 02 +subsystem: api +tags: [autotask, backblaze-b2, postgres, vitest, tdd] + +# Dependency graph +requires: + - phase: 15-data-model-detection-ticket-evidence + provides: indicators table (migration 097), phishing-detector core, EVID-01 ticket evidence capture +provides: + - AutotaskClient.getAttachmentContent() — fetches full base64 `.eml` attachment content via the items[0] convention + - EML_OBJECT_KEY_REGEX + parameterized presignDownload/presignUpload/downloadToBuffer for storing raw `.eml` bytes in B2 under a path-traversal-safe key shape + - migrations/099_indicators_metadata.sql — indicators.metadata JSONB column, applied to dev DB +affects: [16-03-eml-mime-parser-orchestrator, 17-mimecast-blast-radius] + +# Tech tracking +tech-stack: + added: [] + patterns: + - "AutotaskClient per-attachment-ID GET reads response.items?.[0] (list-shaped), NOT response.item" + - "B2 client: parallel object-key regex per use-case, never loosen an existing regex; keyRegex is an optional trailing param defaulting to the original" + +key-files: + created: + - lib/services/autotask-client.test.ts + - migrations/099_indicators_metadata.sql + modified: + - lib/services/autotask-client.ts + - lib/services/b2/client.ts + - lib/services/b2/client.test.ts + +key-decisions: + - "Migration 099 applied to the dev DB via direct docker exec (not scripts/apply-migrations.sh), because that script hardcodes MIGRATIONS_DIR=/opt/stacks/pulse/migrations — the main repo's absolute path, not the worktree's — so it could not see the new file from inside this worktree checkout. Used the same real credentials the script itself defaults to (pulse_user/pulse_autotask), not hand-rolled wrong defaults." + +patterns-established: + - "First AutotaskClient unit test file (autotask-client.test.ts) — mocks global.fetch, asserts items vs item envelope shapes" + +requirements-completed: [EVID-03, EVID-04] + +# Metrics +duration: 25min +completed: 2026-07-15 +--- + +# Phase 16 Plan 02: Autotask Attachment Content, B2 EML Key Regex, Indicators Metadata Column Summary + +**AutotaskClient.getAttachmentContent() (items[0] convention), a parallel EML_OBJECT_KEY_REGEX + parameterized B2 key validation, and migration 099 (indicators.metadata JSONB) — the three independent supporting pieces the Plan 03 EML/MIME orchestrator depends on.** + +## Performance + +- **Duration:** ~25 min +- **Started:** 2026-07-15T14:17:00Z +- **Completed:** 2026-07-15T14:24:00Z +- **Tasks:** 3 completed +- **Files modified:** 5 (2 new, 3 modified) + +## Accomplishments +- `AutotaskClient.getAttachmentContent(entityName, entityId, attachmentId)` fetches a single attachment's full base64 content, correctly reading the live `{items:[...]}` response shape (not `{item:...}`) — verified by a new unit test that also proves an `{item:...}`-shaped response yields `null`, guarding against a future refactor copying the wrong sibling convention. +- `EML_OBJECT_KEY_REGEX` added to `lib/services/b2/client.ts` alongside the existing (untouched) `OBJECT_KEY_REGEX`, enforcing `phishing//.eml` and rejecting path traversal. `presignDownload`, `presignUpload`, and `downloadToBuffer` now accept an optional trailing `keyRegex` parameter (defaulting to `OBJECT_KEY_REGEX`), so the existing LogLift call site (`rmm/executor.ts:337`, `presignUpload(objectKey, 1800)`) compiles and behaves identically. +- `migrations/099_indicators_metadata.sql` adds a nullable `indicators.metadata JSONB` column (D-07), applied live to the dev DB — confirmed via `information_schema.columns`. + +## Task Commits + +Each task was committed atomically (TDD tasks have separate test → feat commits): + +1. **Task 1: AutotaskClient.getAttachmentContent()** + - `8be10db` (test) — failing test for items[0] convention + item-shaped-yields-null guard + - `9b65de7` (feat) — implementation +2. **Task 2: EML_OBJECT_KEY_REGEX + parameterized B2 key validation** + - `6de92a5` (test) — failing tests for new regex + keyRegex param + - `8630fd5` (feat) — implementation +3. **Task 3: Migration 099 — indicators.metadata JSONB** + - `0bf37ce` (feat) — migration file + applied to dev DB + +_RED confirmed for both TDD tasks by temporarily removing the implementation and re-running the test suite before restoring it and committing test-then-feat._ + +## Files Created/Modified +- `lib/services/autotask-client.ts` - added `getAttachmentContent()` after `getAttachments()` +- `lib/services/autotask-client.test.ts` - new; first AutotaskClient unit coverage (3 tests) +- `lib/services/b2/client.ts` - added `EML_OBJECT_KEY_REGEX`; parameterized `keyRegex` on presign*/downloadToBuffer +- `lib/services/b2/client.test.ts` - extended with EML_OBJECT_KEY_REGEX + custom-keyRegex presignUpload tests (5 new tests) +- `migrations/099_indicators_metadata.sql` - new; nullable `indicators.metadata JSONB` + COMMENT + +## Decisions Made +- Applied migration 099 to the dev DB via direct `docker exec pulse-postgres psql -U pulse_user -d pulse_autotask` rather than `scripts/apply-migrations.sh`, because that script's `MIGRATIONS_DIR` is hardcoded to the main repo's absolute path and cannot see files that only exist in this worktree checkout. Used the same real project credentials (`pulse_user`/`pulse_autotask`) the script itself would use — not the script's local-non-Docker fallback defaults. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 3 - Blocking] scripts/apply-migrations.sh could not see the new migration file from inside the worktree** +- **Found during:** Task 3 (migration 099 apply step) +- **Issue:** The script hardcodes `MIGRATIONS_DIR="/opt/stacks/pulse/migrations"` — the main repo's path, not this worktree's `.claude/worktrees/agent-a2b7d8cafac0f4270/migrations/`. Running it reported "Migration file 099_indicators_metadata.sql not found" even though the file existed in the worktree. +- **Fix:** Applied the migration directly with `docker exec -i pulse-postgres psql -U pulse_user -d pulse_autotask < migrations/099_indicators_metadata.sql`, using the identical real credentials the script defaults to for Docker mode (not the script's local/non-Docker psql fallback defaults, which are wrong for this project). +- **Files modified:** none (operational step only) +- **Verification:** `information_schema.columns` confirms `indicators.metadata` is `jsonb` on the dev DB. +- **Committed in:** 0bf37ce (Task 3 commit, migration file only — no code changed by this deviation) + +--- + +**Total deviations:** 1 auto-fixed (1 blocking) +**Impact on plan:** No scope creep — same migration content and same live-DB outcome the plan specified; only the apply mechanism differed due to a pre-existing worktree/script path mismatch. + +## Issues Encountered +None beyond the deviation above. + +## User Setup Required +None - no external service configuration required. + +## Next Phase Readiness +- `getAttachmentContent()`, `EML_OBJECT_KEY_REGEX`, and `indicators.metadata` are all available for Plan 03's EML/MIME parser orchestrator to consume. +- `rmm/executor.ts`'s existing `presignUpload(objectKey, 1800)` call site still type-checks unchanged (verified via `npx tsc --noEmit`). +- No blockers. + +## Self-Check: PASSED + +All created/modified files verified present on disk; all task and metadata commit hashes verified present in git log. + +--- +*Phase: 16-eml-mime-evidence-parser* +*Completed: 2026-07-15* diff --git a/lib/services/autotask-client.test.ts b/lib/services/autotask-client.test.ts new file mode 100644 index 0000000..cc50a0d --- /dev/null +++ b/lib/services/autotask-client.test.ts @@ -0,0 +1,73 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { AutotaskClient } from './autotask-client'; +import type { AutotaskConfig } from '@/lib/types/autotask'; + +const FIXTURE_CONFIG: AutotaskConfig = { + apiUrl: 'https://webservices.autotask.net/atservicesrest/v1.0', + username: 'fixture@example.com', + password: 'fixture-secret', + apiIntegrationCode: 'FIXTURE-CODE', +}; + +function jsonResponse(body: unknown, ok = true, status = 200): Response { + return { + ok, + status, + statusText: ok ? 'OK' : 'Error', + text: async () => JSON.stringify(body), + } as unknown as Response; +} + +describe('AutotaskClient.getAttachmentContent', () => { + const originalFetch = global.fetch; + + beforeEach(() => { + global.fetch = vi.fn(); + }); + + afterEach(() => { + global.fetch = originalFetch; + vi.restoreAllMocks(); + }); + + it('returns items[0] with populated base64 data when the API returns an items-shaped envelope', async () => { + const attachment = { id: 555, fullPath: 'rfc.eml', title: 'rfc.eml', data: 'YmFzZTY0LWNvbnRlbnQ=' }; + (global.fetch as unknown as ReturnType).mockResolvedValue( + jsonResponse({ items: [attachment] }) + ); + + const client = new AutotaskClient(FIXTURE_CONFIG); + const result = await client.getAttachmentContent('Tickets', 12345, 555); + + expect(result).toEqual(attachment); + expect(global.fetch).toHaveBeenCalledWith( + `${FIXTURE_CONFIG.apiUrl}/Tickets/12345/Attachments/555`, + expect.objectContaining({ method: 'GET' }) + ); + }); + + it('returns null when the API returns an empty items array', async () => { + (global.fetch as unknown as ReturnType).mockResolvedValue( + jsonResponse({ items: [] }) + ); + + const client = new AutotaskClient(FIXTURE_CONFIG); + const result = await client.getAttachmentContent('Tickets', 12345, 999); + + expect(result).toBeNull(); + }); + + it('returns null (not the attachment) when the API returns an {item:...}-shaped response', async () => { + // Guards against a future refactor copying uploadAttachment's `.item` + // convention onto this method — the live shape is `.items`. + const attachment = { id: 555, fullPath: 'rfc.eml', title: 'rfc.eml', data: 'YmFzZTY0LWNvbnRlbnQ=' }; + (global.fetch as unknown as ReturnType).mockResolvedValue( + jsonResponse({ item: attachment }) + ); + + const client = new AutotaskClient(FIXTURE_CONFIG); + const result = await client.getAttachmentContent('Tickets', 12345, 555); + + expect(result).toBeNull(); + }); +}); diff --git a/lib/services/autotask-client.ts b/lib/services/autotask-client.ts index 127cda5..bb8492f 100644 --- a/lib/services/autotask-client.ts +++ b/lib/services/autotask-client.ts @@ -435,6 +435,26 @@ export class AutotaskClient { return response.items || []; } + // Fetches a single attachment's full content (base64 `data` populated). + // Confirmed live: the per-attachment-ID GET returns `{items:[...]}` + // (list-shaped), NOT `{item:...}` like getEntityById/uploadAttachment — + // do not "fix" this to read response.item, that would silently return + // undefined content. + async getAttachmentContent( + entityName: string, + entityId: number, + attachmentId: number + ): Promise { + const url = `${this.config.apiUrl}/${entityName}/${entityId}/Attachments/${attachmentId}`; + + const response = await this.makeApiCall>(url, { + method: 'GET', + headers: this.getAuthHeaders(), + }); + + return response.items?.[0] ?? null; + } + // Time Entries specific methods async getTimeEntriesByResource(resourceId: number): Promise { return this.queryEntity('TimeEntries', { diff --git a/lib/services/b2/client.test.ts b/lib/services/b2/client.test.ts index b5f29a5..e55be05 100644 --- a/lib/services/b2/client.test.ts +++ b/lib/services/b2/client.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { OBJECT_KEY_REGEX, + EML_OBJECT_KEY_REGEX, presignDownload, presignUpload, B2InvalidObjectKeyError, @@ -117,6 +118,58 @@ describe('presignDownload + presignUpload', () => { }); }); +describe('EML_OBJECT_KEY_REGEX', () => { + it('accepts phishing//.eml', () => { + expect( + EML_OBJECT_KEY_REGEX.test('phishing/ba03268b-5528-4dde-ad76-867523446ecd/555.eml') + ).toBe(true); + expect(EML_OBJECT_KEY_REGEX.test('phishing/report_1/attachment_1.eml')).toBe(true); + }); + + it('rejects path traversal', () => { + expect(EML_OBJECT_KEY_REGEX.test('phishing/../evil.eml')).toBe(false); + expect(EML_OBJECT_KEY_REGEX.test('phishing/report/../../escape.eml')).toBe(false); + }); + + it('rejects wrong extension and the LogLift shape', () => { + expect(EML_OBJECT_KEY_REGEX.test('phishing/a/b.json')).toBe(false); + expect( + EML_OBJECT_KEY_REGEX.test( + 'ba03268b-5528-4dde-ad76-867523446ecd/unknown-server/eventlogs_20251202_173301.json.gz' + ) + ).toBe(false); + }); +}); + +describe('presignUpload with a custom keyRegex', () => { + it('succeeds for a valid .eml key validated against EML_OBJECT_KEY_REGEX', () => { + const url = presignUpload( + 'phishing/report_1/attachment_1.eml', + 1800, + FIXTURE_CFG, + EML_OBJECT_KEY_REGEX + ); + expect(url).toContain('X-Amz-Expires=1800'); + }); + + it('throws B2InvalidObjectKeyError for a LogLift-shaped key when validated against EML_OBJECT_KEY_REGEX', () => { + expect(() => + presignUpload( + 'site/host/eventlogs_20260502_120000.json.gz', + 1800, + FIXTURE_CFG, + EML_OBJECT_KEY_REGEX + ) + ).toThrow(B2InvalidObjectKeyError); + }); + + it('still validates against OBJECT_KEY_REGEX by default (existing LogLift call sites unchanged)', () => { + expect(() => + presignUpload('phishing/report_1/attachment_1.eml', 1800, FIXTURE_CFG) + ).toThrow(B2InvalidObjectKeyError); + }); +}); + describe('deriveSigningKey', () => { it('produces a 32-byte HMAC-SHA256 chain', () => { const k = _B2_INTERNALS.deriveSigningKey( diff --git a/lib/services/b2/client.ts b/lib/services/b2/client.ts index e424201..033ced8 100644 --- a/lib/services/b2/client.ts +++ b/lib/services/b2/client.ts @@ -31,6 +31,16 @@ export const MAX_DOWNLOAD_BYTES = 25 * 1024 * 1024; // 25 MB export const OBJECT_KEY_REGEX = /^[A-Za-z0-9_-]+\/[A-Za-z0-9_.-]+\/eventlogs_[0-9_]+\.json\.gz$/; +/** + * Object-key shape for raw `.eml` evidence uploads (Phase 16 / D-05): + * `phishing/{reportId}/{attachmentId}.eml`. This is a SEPARATE regex from + * OBJECT_KEY_REGEX — per the B2 evidence skill doc, never loosen the + * existing LogLift guard to accommodate a new shape. Path-traversal safe: + * each segment is restricted to `[A-Za-z0-9_-]+`, so `..` cannot appear. + */ +export const EML_OBJECT_KEY_REGEX = + /^phishing\/[A-Za-z0-9_-]+\/[A-Za-z0-9_-]+\.eml$/; + export class B2NotConfiguredError extends Error { constructor() { super( @@ -145,18 +155,20 @@ function presign(params: PresignParams): string { export function presignDownload( objectKey: string, expiresInSeconds = 600, - cfg: B2Config = getB2Config() + cfg: B2Config = getB2Config(), + keyRegex: RegExp = OBJECT_KEY_REGEX ): string { - if (!OBJECT_KEY_REGEX.test(objectKey)) throw new B2InvalidObjectKeyError(objectKey); + if (!keyRegex.test(objectKey)) throw new B2InvalidObjectKeyError(objectKey); return presign({ method: 'GET', objectKey, expiresInSeconds, config: cfg }); } export function presignUpload( objectKey: string, expiresInSeconds = 1800, - cfg: B2Config = getB2Config() + cfg: B2Config = getB2Config(), + keyRegex: RegExp = OBJECT_KEY_REGEX ): string { - if (!OBJECT_KEY_REGEX.test(objectKey)) throw new B2InvalidObjectKeyError(objectKey); + if (!keyRegex.test(objectKey)) throw new B2InvalidObjectKeyError(objectKey); return presign({ method: 'PUT', objectKey, expiresInSeconds, config: cfg }); } @@ -166,9 +178,10 @@ export function presignUpload( */ export async function downloadToBuffer( objectKey: string, - cfg: B2Config = getB2Config() + cfg: B2Config = getB2Config(), + keyRegex: RegExp = OBJECT_KEY_REGEX ): Promise { - const url = presignDownload(objectKey, 600, cfg); + const url = presignDownload(objectKey, 600, cfg, keyRegex); const res = await fetch(url); if (!res.ok) { const text = await res.text().catch(() => ''); diff --git a/migrations/099_indicators_metadata.sql b/migrations/099_indicators_metadata.sql new file mode 100644 index 0000000..884be84 --- /dev/null +++ b/migrations/099_indicators_metadata.sql @@ -0,0 +1,16 @@ +-- ============================================================================= +-- indicators.metadata JSONB (Phase 16 — D-07) +-- ============================================================================= +-- Lets an attachment-hash indicator carry filename/content-type/size, or a +-- URL indicator carry which message part it came from, without duplicating +-- that context into the parent messages row. +-- +-- Nullable, no default — existing rows (there are none yet for this table) +-- are unaffected; this is a pure additive change. +-- ============================================================================= + +ALTER TABLE indicators + ADD COLUMN IF NOT EXISTS metadata JSONB; + +COMMENT ON COLUMN indicators.metadata IS + 'Per-indicator context (e.g. attachment filename/content-type/size for attachment-hash indicators, or source message part for URL indicators). Nullable — no backfill required.';