Description

getHealthSnapshot() merges arbitrary plugin-provided buildChannelSummary() objects into the health record even when includeSensitive is false.

• buildChannelSummary() is typed to return Record<string, unknown> (arbitrary keys).
• The implementation merges it directly into the response via { ...snapshot, ...summary }.
• When includeSensitive is false, the code only deletes record.probe.
• Any other sensitive fields placed by a plugin summary (e.g., token, webhookUrl, channelSecret, stack traces, internal URLs) will be returned to non-admin callers.

Vulnerable code:

const record = summary && typeof summary === "object"
  ? ({ ...snapshot, ...summary } as ChannelAccountHealthSummary)
  : ({ ...snapshot, accountId, configured } satisfies ChannelAccountHealthSummary);
...
if (!includeSensitive) {
  delete record.probe;
}

Because gateway health responses set includeSensitive based on scopes, non-admin clients can still receive these unredacted plugin summary fields.

Recommendation

When includeSensitive === false, enforce an allowlist for fields that may leave the process.

Options:

1. Project/sanitize plugin summaries before merging (recommended):

const safeSummary = includeSensitive
  ? summary
  : projectSafeChannelAccountSnapshotFields(summary as Record<string, unknown>);

const record = summary && typeof summary === "object"
  ? ({ ...snapshot, ...safeSummary } as ChannelAccountHealthSummary)
  : ({ ...snapshot, accountId, configured } satisfies ChannelAccountHealthSummary);

if (!includeSensitive) {
  ​// also defensively drop any known-sensitive keys that might still exist
  delete record.probe;
  delete (record as any).token;
  delete (record as any).secret;
  delete (record as any).webhookUrl;
}

2. Change the plugin API so buildChannelSummary receives includeSensitive and must return either a safe subset or a { safe, sensitive } structure; only include sensitive for admin callers.

Additionally, consider preventing plugin summaries from overwriting core snapshot fields (e.g., merge as { ...summary, ...snapshot } or explicitly set critical fields after merge).

1. src/commands/health.ts, line 391-399 (link)

   Runtime fields dropped when buildChannelSummary returns falsy

   The { ...snapshot, ...summary } path correctly preserves runtime fields (connected, lastConnectedAt) when a plugin returns an object summary. But the else branch reconstructs the record from scratch — { accountId, configured, probe, lastProbeAt } — discarding whatever runtime state was spread into snapshot. If any plugin's buildChannelSummary returns null/undefined/falsy (or if a future plugin omits it entirely), the runtime fields are silently dropped even though they were captured moments earlier.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/commands/health.ts
   Line: 391-399
   
   Comment:
   **Runtime fields dropped when `buildChannelSummary` returns falsy**
   
   The `{ ...snapshot, ...summary }` path correctly preserves runtime fields (`connected`, `lastConnectedAt`) when a plugin returns an object summary. But the else branch reconstructs the record from scratch — `{ accountId, configured, probe, lastProbeAt }` — discarding whatever runtime state was spread into `snapshot`. If any plugin's `buildChannelSummary` returns `null`/`undefined`/falsy (or if a future plugin omits it entirely), the runtime fields are silently dropped even though they were captured moments earlier.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.