Reported by an adversarial review pass while reviewing an unrelated PR. This is PRE-EXISTING in shipped code, not introduced by that PR, so it is being routed here rather than described in a public comment.

Summary

A renderer-supplied identifier is passed into a filesystem path join in the Insights IPC handlers with no validation, permitting relative traversal outside the intended directory.

Detail

src/main/ipc/insights-handlers.ts forwards the renderer's runId unchanged to getInsightsReport(runId) and getInsightsKpis(runId) in src/main/insights-runner.ts, each of which does:

join(getInsightsDir(), runId, '<fixed filename>')

There is no charset or shape validation on runId. Confirmed with a Node probe against a throwaway directory tree replicating the join verbatim: a relative runId containing .. segments (both / and \ separators) resolves outside the insights directory and reads the file there.

Constraints that bound the impact:

• Only two fixed filenames are ever appended, so an arbitrary file cannot be named directly.
• An absolute-path runId does NOT escape: Node's path.join (unlike path.resolve) does not reset on an absolute later segment. The primitive is relative-traversal only.
• Exploitation requires a renderer-side code-execution primitive first. contextIsolation: true and nodeIntegration: false are set (src/main/index.ts), so this is defense-in-depth rather than a zero-click vector.

Byte-compared both functions against beta — identical, confirming this predates the PR under review. The newly added memberRunIds field does not widen it: those ids are generated internally, never renderer-supplied.

Suggested fix

This repository already has the correct pattern for this exact bug class. src/main/ipc/notes-handlers.ts validates its renderer-supplied id against a strict charset before any filesystem use, added specifically as a path-traversal guard. Applying the equivalent guard to runId in the Insights handlers closes it. Run ids are produced by generateRunId() and match a narrow known shape, so an allowlist is straightforward and cannot break legitimate callers.

A companion hardening in the same area: getProfileConfigDir(id) does not call the isValidProfileId guard defined immediately above it, and the insights:run handler passes a raw renderer profileId into resolveInsightsAccount, which gates only on existsSync and no regex.

Reported by

Adversarial review pass (ADR-009) on the branch for issue #191. No public disclosure has been made: the PR verdict records only that a pre-existing finding was routed privately, with no component, mechanism or repro.