|
| 1 | +# Investigation: Sentry Telemetry Scope and Privacy Audit (#204) |
| 2 | + |
| 3 | +## Summary |
| 4 | +The current implementation captured too much data by default (including broad MCP instrumentation and PII-heavy tagging). The fix narrows telemetry to internal runtime failures only, keeps tracing enabled for observability, disables MCP input/output capture, and adds payload scrubbing/redaction. |
| 5 | + |
| 6 | +## Symptoms |
| 7 | +- Sentry was initialized with `sendDefaultPii: true` and `tracesSampleRate: 1.0`. |
| 8 | +- MCP server was wrapped with `wrapMcpServerWithSentry`, enabling broad per-call instrumentation. |
| 9 | +- Logger default sent all `error` logs to Sentry, including user-domain failures. |
| 10 | +- Sentry tags included sensitive environment/system values (HOME, USER, PATH, Xcode paths). |
| 11 | +- Privacy docs understated actual collection scope. |
| 12 | + |
| 13 | +## Investigation Log |
| 14 | + |
| 15 | +### Phase 1 - Issue and baseline audit |
| 16 | +**Hypothesis:** Telemetry defaults and wrapper behavior were broader than documented. |
| 17 | +**Findings:** Issue #204 correctly identified mismatch between docs and implementation. |
| 18 | +**Evidence:** |
| 19 | +- GitHub issue: https://github.com/getsentry/XcodeBuildMCP/issues/204 |
| 20 | +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/src/utils/sentry.ts` |
| 21 | +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/src/server/server.ts` |
| 22 | +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/src/utils/logger.ts` |
| 23 | +**Conclusion:** Confirmed. |
| 24 | + |
| 25 | +### Phase 2 - Runtime path tracing |
| 26 | +**Hypothesis:** User-domain tool failures were reaching Sentry through logger defaults. |
| 27 | +**Findings:** `log('error', ...)` implicitly captured to Sentry unless overridden; many tool/runtime paths emit user-domain errors at error level. |
| 28 | +**Evidence:** |
| 29 | +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/src/utils/logger.ts` |
| 30 | +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/src/utils/build-utils.ts` |
| 31 | +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/src/runtime/tool-invoker.ts` |
| 32 | +**Conclusion:** Confirmed. |
| 33 | + |
| 34 | +### Phase 3 - Version and docs alignment |
| 35 | +**Hypothesis:** Sentry SDK was not at latest patch and docs were not aligned with behavior. |
| 36 | +**Findings:** `@sentry/node` moved from `^10.37.0` to latest `10.38.0` and docs were updated to match internal-only policy. |
| 37 | +**Evidence:** |
| 38 | +- Command: `npm view @sentry/node version` returned `10.38.0` |
| 39 | +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/package.json` |
| 40 | +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/docs/PRIVACY.md` |
| 41 | +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/docs/CONFIGURATION.md` |
| 42 | +- `/Users/cameroncooke/.codex/worktrees/a59a/XcodeBuildMCP/README.md` |
| 43 | +**Conclusion:** Confirmed. |
| 44 | + |
| 45 | +## Root Cause |
| 46 | +Three defaults combined to over-collect telemetry: |
| 47 | +1. `sendDefaultPii: true` and tracing at 100% in Sentry init. |
| 48 | +2. `wrapMcpServerWithSentry` around the MCP server. |
| 49 | +3. Logger behavior that captured every `error` log to Sentry by default. |
| 50 | + |
| 51 | +This effectively blurred boundaries between internal platform failures and user-domain build/config/runtime failures, and increased risk of sensitive metadata leakage. |
| 52 | + |
| 53 | +## Changes Implemented |
| 54 | +1. Sentry initialization hardened (`sendDefaultPii: false`, `tracesSampleRate: 1.0`, breadcrumbs disabled, `beforeSend` scrubbing/redaction). |
| 55 | +2. Sensitive tags/context removed (no env dumps, no HOME/USER/PATH/Xcode path tagging). |
| 56 | +3. Restored MCP wrapper with explicit safe options (`recordInputs: false`, `recordOutputs: false`) to keep tool-level observability without payload capture. |
| 57 | +4. Logger changed to explicit opt-in capture only (`context.sentry === true`). |
| 58 | +5. Internal boundary capture retained only where appropriate (startup/shutdown/fatal daemon internal errors). |
| 59 | +6. Added tests for explicit capture policy and path redaction. |
| 60 | +7. Updated privacy/config/README/architecture docs and changelog. |
| 61 | + |
| 62 | +## Eliminated Hypotheses |
| 63 | +- "Only MCP-level faults are captured today": Eliminated (not true before this patch due to logger defaults and wrapper). |
| 64 | +- "Docs accurately reflected telemetry scope": Eliminated. |
| 65 | + |
| 66 | +## Recommendations |
| 67 | +1. Keep Sentry capture explicit and centralized to internal runtime boundaries. |
| 68 | +2. Avoid adding environment or filesystem metadata to telemetry tags. |
| 69 | +3. Preserve redaction tests to prevent regressions. |
| 70 | +4. Continue documenting telemetry scope in user-facing docs whenever behavior changes. |
| 71 | + |
| 72 | +## Preventive Measures |
| 73 | +- CI should keep redaction and logger policy tests running by default. |
| 74 | +- Any future telemetry additions should require explicit privacy review with docs update in same PR. |
| 75 | + |
| 76 | +## Validation |
| 77 | +All relevant quality checks were executed after changes: |
| 78 | + |
| 79 | +- `npm run format:check` ✅ |
| 80 | +- `npm run lint` ✅ |
| 81 | +- `npm run typecheck` ✅ |
| 82 | +- `npm run build` ✅ |
| 83 | +- `npm test` ✅ (117 test files passed; 1274 tests passed, 15 skipped) |
| 84 | + |
| 85 | +Notes: |
| 86 | +- Test output includes expected stderr from negative-path parser tests in `src/utils/__tests__/nskeyedarchiver-parser.test.ts`; test run still passed. |
| 87 | + |
| 88 | +## Addendum: MCP Wrapper Capture Semantics (verified against SDK 10.38.0) |
| 89 | +- `wrapMcpServerWithSentry` resolves options as: |
| 90 | + - `recordInputs: options?.recordInputs ?? sendDefaultPii` |
| 91 | + - `recordOutputs: options?.recordOutputs ?? sendDefaultPii` |
| 92 | +- For MCP request spans, `tools/call` and `prompts/get` arguments are added as `mcp.request.argument.*` attributes when `recordInputs=true`. |
| 93 | +- Tool/prompt results are added as `mcp.tool.result*` / `mcp.prompt.result*` attributes when `recordOutputs=true`. |
| 94 | +- Built-in MCP PII filtering in the SDK only strips network-level fields (`client.address`, `client.port`, `mcp.resource.uri`) when `sendDefaultPii=false`; it does not by itself scrub arbitrary tool argument/output payloads. |
| 95 | + |
| 96 | +Evidence (source-inspected package build): |
| 97 | +- `@sentry/core@10.38.0` `build/cjs/integrations/mcp-server/index.js` |
| 98 | +- `@sentry/core@10.38.0` `build/cjs/integrations/mcp-server/methodConfig.js` |
| 99 | +- `@sentry/core@10.38.0` `build/cjs/integrations/mcp-server/resultExtraction.js` |
| 100 | +- `@sentry/core@10.38.0` `build/cjs/integrations/mcp-server/piiFiltering.js` |
| 101 | + |
| 102 | +## Addendum: Live Validation (2026-02-12) |
| 103 | + |
| 104 | +### Findings |
| 105 | +- Runtime config/dependency tags are being attached to issues when events are captured after bootstrap context is set. |
| 106 | +- Example: issue `XCODEBUILDMCP-6` includes `runtime.mode`, `xcode.version`, `xcode.xcodebuild_path`, `axe.source`, and config tags. |
| 107 | +- Startup-phase config parse warnings can occur before full runtime context is attached, so those earlier events may not show the full tag set. |
| 108 | +- MCP wrapper instrumentation is active in-process: |
| 109 | + - Local debug output shows sampled MCP spans for `initialize`, `notifications/initialized`, and `tools/call session_show_defaults`. |
| 110 | + - Local exporter reports spans exported. |
| 111 | +- Despite local span export, Sentry project query for spans currently returns `count() = 0` in the tested time window. |
| 112 | + |
| 113 | +### Evidence |
| 114 | +- Local MCP call validation: |
| 115 | + - `session_show_defaults` invoked over stdio client; server started successfully. |
| 116 | +- Local in-memory instrumentation validation: |
| 117 | + - Debug logs show: |
| 118 | + - `Starting sampled root span op: mcp.server` |
| 119 | + - `Finishing ... tools/call session_show_defaults` |
| 120 | + - `SpanExporter exported 3 spans` |
| 121 | +- Sentry MCP queries: |
| 122 | + - Spans in last hour: `count() = 0` |
| 123 | + - Transactions in last hour: `count() = 0` |
| 124 | + - Trace for issue `XCODEBUILDMCP-6`: `Total Spans: 0, Errors: 1` |
| 125 | + |
| 126 | +## Addendum: MCP View + Logs Explorer Deep Dive (2026-02-12) |
| 127 | + |
| 128 | +### Scope |
| 129 | +Investigated two active symptoms: |
| 130 | +- MCP tools are not visible in Sentry MCP view. |
| 131 | +- Logs are not visible in Sentry Logs Explorer. |
| 132 | + |
| 133 | +### Key Findings |
| 134 | +- Error events are ingesting correctly in the target project. |
| 135 | + - Sentry query: errors in last 24h = `11`. |
| 136 | +- Logs and spans datasets remain empty in the same project/time windows. |
| 137 | + - Sentry query: logs in last 24h/7d = `0`. |
| 138 | + - Sentry query: spans in last 24h/14d = `0`. |
| 139 | + - Sentry query: transactions in last 14d/15m = `0`. |
| 140 | +- SDK-side emission is working for both logs and transactions. |
| 141 | + - Direct probe emitted: |
| 142 | + - `Sentry.logger.info('envelope logger probe ...')` |
| 143 | + - `Sentry.startSpan({ forceTransaction: true, ... })` |
| 144 | + - Runtime instrumentation confirmed envelope item types sent: |
| 145 | + - `ENVELOPE_ITEM_TYPES log` |
| 146 | + - `ENVELOPE_ITEM_TYPES transaction` |
| 147 | + - plus expected `event`/`session` items. |
| 148 | +- Despite emitted envelopes, Sentry queries still return zero logs/spans/transactions. |
| 149 | + - Strongly indicates an ingestion/storage/configuration issue outside current app code path. |
| 150 | + |
| 151 | +### Code-Path Validation |
| 152 | +- MCP wrapper is enabled with safe options: |
| 153 | + - `src/server/server.ts:72` uses `wrapMcpServerWithSentry(..., { recordInputs: false, recordOutputs: false })`. |
| 154 | +- Sentry logs pipeline is enabled: |
| 155 | + - `src/utils/sentry.ts:282` sets `enableLogs: true`. |
| 156 | + - `src/utils/sentry.ts:286` sets `beforeSendLog: redactLog`. |
| 157 | +- Logger forwards only explicit opt-in internal logs: |
| 158 | + - `src/utils/logger.ts:56` (`context?.sentry === true`). |
| 159 | + - `src/utils/logger.ts:236` fallback uses `captureMessage` only if logger method is unavailable. |
| 160 | +- Runtime split is real: |
| 161 | + - Daemon handles `tool.invoke` requests (`src/daemon/daemon-server.ts:117`), including `runtime: 'daemon'` (`src/daemon/daemon-server.ts:128`). |
| 162 | + - CLI paths route many invocations through daemon (`src/runtime/tool-invoker.ts:192`). |
| 163 | + - MCP wrapper only covers stdio MCP server runtime (`src/server/server.ts:72`). |
| 164 | + |
| 165 | +### Root Cause Assessment (Current Confidence) |
| 166 | +- Most likely primary blocker: Sentry-side configuration/entitlement/pipeline for traces and logs in this project/org (not client emission). |
| 167 | +- Secondary (not primary) code risk: |
| 168 | + - `process.exit(...)` without explicit `Sentry.flush/close` in shutdown paths can still drop buffered telemetry in some paths: |
| 169 | + - `src/server/start-mcp-server.ts:68` |
| 170 | + - `src/server/start-mcp-server.ts:83` |
| 171 | + - `src/daemon.ts:303` |
| 172 | + - `src/daemon.ts:310` |
| 173 | + - `src/daemon.ts:402` |
| 174 | + |
| 175 | +### Eliminated Hypotheses |
| 176 | +- "MCP wrapper is removed or disabled." Eliminated. |
| 177 | +- "Logs are not being captured by SDK at all." Eliminated (capture hooks + envelope inspection confirm capture/send). |
| 178 | +- "Transactions are not being created by SDK at all." Eliminated (manual forced transaction emitted and sent). |
| 179 | + |
| 180 | +### Recommended Next Steps |
| 181 | +1. Verify project-level traces/logs ingestion settings in Sentry (`sentry/xcodebuildmcp`) and any org-level sampling/filtering rules dropping transactions/logs. |
| 182 | +2. Verify account/product entitlement for Logs and Performance on this project. |
| 183 | +3. Add explicit shutdown drain in app code (`Sentry.flush`/`Sentry.close`) before `process.exit(...)` to reduce telemetry loss on fast shutdown. |
| 184 | +4. Keep MCP-view expectation scoped to MCP stdio runtime; add daemon-specific spans if daemon tool-call observability is required in traces. |
0 commit comments