chore: merge executor worktree (worktree-agent-a2b7d8cafac0f4270)

.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/<reportId>/<attachmentId>.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*

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<typeof vi.fn>).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<typeof vi.fn>).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<typeof vi.fn>).mockResolvedValue(
+      jsonResponse({ item: attachment })
+    );
+
+    const client = new AutotaskClient(FIXTURE_CONFIG);
+    const result = await client.getAttachmentContent('Tickets', 12345, 555);
+
+    expect(result).toBeNull();
+  });
+});

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<Attachment | null> {
+    const url = `${this.config.apiUrl}/${entityName}/${entityId}/Attachments/${attachmentId}`;
+
+    const response = await this.makeApiCall<ApiResponse<Attachment>>(url, {
+      method: 'GET',
+      headers: this.getAuthHeaders(),
+    });
+
+    return response.items?.[0] ?? null;
+  }
+
   // Time Entries specific methods
   async getTimeEntriesByResource(resourceId: number): Promise<AutotaskTimeEntry[]> {
     return this.queryEntity<AutotaskTimeEntry>('TimeEntries', {

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/<reportId>/<attachmentId>.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(

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<Buffer> {
-  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(() => '');

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.';