fix(desktop): local diagnostics export + owner-only (0600) log file#9106
Merged
eulicesl merged 2 commits intoJul 6, 2026
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 6 files
Confidence score: 3/5
desktop/macos/Desktop/Sources/FeedbackView.swiftrunssaveDiagnosticsLocally()synchronous file read/serialize/write work on the main actor, so the feedback UI can stall during save and feel frozen for users—move this I/O off the main actor (or split into async/background work) before merging.desktop/macos/Desktop/Sources/DesktopDiagnosticsManager.swiftreads the full log into memory before trimming, which can make diagnostics export slow or hang for large logs and degrade reliability under heavy usage—switch to bounded tail reading (bytes/chunks) so export cost stays predictable.desktop/macos/Desktop/Sources/Logger.swiftmay permanently skip permission normalization after one failure because the one-time guard is set too early, leaving logs with weaker-than-intended file permissions—only mark hardening as done when 0600 normalization actually succeeds (or allow retries).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the macOS desktop app’s diagnostics and logging by (1) enforcing owner-only permissions on the on-disk /tmp/omi*.log file and (2) adding an offline “Save Diagnostics…” export that writes a redacted diagnostics bundle to a user-selected location.
Changes:
- Add log permission normalization to ensure the
/tmp/omi*.logfile is0600and provide a single source of truth for the log path. - Add an offline diagnostics export flow (UI entrypoint + writer) that bundles metadata, sanitized health snapshots, and a redacted log tail.
- Add unit tests covering log permission handling and diagnostics export/redaction behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| desktop/macos/Desktop/Tests/LoggerPermissionsTests.swift | New tests pin owner-only log file creation and tightening behavior. |
| desktop/macos/Desktop/Tests/DiagnosticsExportTests.swift | New tests verify offline bundle writing, redaction behavior, sanitized snapshots, and missing-log handling. |
| desktop/macos/Desktop/Sources/Logger.swift | Adds log path accessor and owner-only permission enforcement on first write. |
| desktop/macos/Desktop/Sources/FeedbackView.swift | Adds “Save Diagnostics…” action and uses shared log path for Sentry attachments. |
| desktop/macos/Desktop/Sources/DesktopDiagnosticsManager.swift | Implements offline diagnostics text/bundle writer with log-tail redaction. |
| desktop/macos/changelog/unreleased/20260705-diagnostics-export-log-hygiene.json | Changelog entry for diagnostics export + log permission hardening. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
eulicesl
added a commit
to eulicesl/omi-fork
that referenced
this pull request
Jul 6, 2026
- Logger: latch the owner-only permission guard from ensureLogFileOwnerOnly's result so a failed 0600 normalization is retried, not skipped permanently. - DesktopDiagnosticsManager: read only a bounded 512KB tail of the log via FileHandle instead of loading the whole file, keeping export latency/memory predictable on large logs (lenient UTF-8 decode drops a partial first line). - FeedbackView: move the diagnostics bundle build/write off the main thread so a large log can't hang the UI; reveal in Finder back on main. - Test: add a >512KB log case asserting the bounded read returns the tail, not the head. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
eulicesl
force-pushed
the
fix/desktop-diagnostics-export
branch
from
9e07207 to
c593e47
Compare
…e owner-only BL-024 (SET-06): the app log in the shared /tmp directory was created with default (world-readable) permissions. Logger now creates the file 0600 and tightens any pre-existing looser log once per process, so other local users can no longer read it. Adds a single source-of-truth `omiLogFilePath()`. BL-023 (SET-03): "Report Issue" was Sentry-only, useless offline or with the crash reporter off. Adds a "Save Diagnostics…" action that writes a redacted, offline diagnostics bundle (app/OS metadata + sanitized health snapshots + a token-redacted log tail) to a user-chosen location and reveals it in Finder. Tests: LoggerPermissionsTests (0600 on create + tighten-existing); DiagnosticsExportTests (offline write, token/JWT/bearer redaction, PII-free snapshots, missing-log handling). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
eulicesl
added a commit
to eulicesl/omi-fork
that referenced
this pull request
Jul 6, 2026
- Logger: latch the owner-only permission guard from ensureLogFileOwnerOnly's result so a failed 0600 normalization is retried, not skipped permanently. - DesktopDiagnosticsManager: read only a bounded 512KB tail of the log via FileHandle instead of loading the whole file, keeping export latency/memory predictable on large logs (lenient UTF-8 decode drops a partial first line). - FeedbackView: move the diagnostics bundle build/write off the main thread so a large log can't hang the UI; reveal in Finder back on main. - Test: add a >512KB log case asserting the bounded read returns the tail, not the head. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
eulicesl
force-pushed
the
fix/desktop-diagnostics-export
branch
from
c593e47 to
54d5a6b
Compare
… + Copilot) - Logger.ensureLogFileOwnerOnly: refuse to adopt a pre-created log path that isn't a regular file owned by us. lstat-check the node; a symlink, non-regular node, or another user's file is removed and recreated 0600, so in world- writable /tmp we never chmod a symlink target or write through a link into a file we don't control (Copilot security finding). - Logger.writeToLogFile: latch the owner-only guard from ensureLogFileOwnerOnly's result so a failed normalization is retried, not skipped permanently. - DesktopDiagnosticsManager: read only a bounded 512KB tail of the log via FileHandle instead of loading the whole file, keeping export latency/memory predictable on large logs (lenient UTF-8 decode drops a partial first line). - FeedbackView: move the diagnostics bundle build/write off the main thread so a large log can't hang the UI; reveal in Finder back on main. - Tests: symlink-rejection case for the log helper; a >512KB log case asserting the bounded read returns the tail, not the head. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
eulicesl
force-pushed
the
fix/desktop-diagnostics-export
branch
from
54d5a6b to
e34c1cd
Compare
eulicesl
added a commit
that referenced
this pull request
Jul 6, 2026
…rt (#9130) ## What Extend the local diagnostics-export redactor to catch secret shapes it previously missed (follow-up to the redaction gap noted during the diagnostics-export work in #9106). Adds redaction patterns for: - `Authorization: Basic <base64>` credentials → `Basic [redacted]` - Bare OpenAI-style API keys (`sk-…`) → `sk-[redacted]` ## Why The offline "Save Diagnostics…" bundle is meant to be safe to share manually, but the redactor only masked JWTs, `Bearer` tokens, and `key=`/`token:` kv pairs — a bare `sk-` key or a `Basic` auth header would have survived into an exported bundle. ## Testing `DiagnosticsExportTests` extended to assert the new shapes are masked; `bash test.sh` (desktop suite) green. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/BasedHardware/omi/pull/9130?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img](https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a){kind=link}
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two desktop diagnostics/log-hygiene fixes (macOS hardening):
/tmp(/tmp/omi-dev.log//tmp/omi.log) was created with default (world-readable) permissions. It's now created0600(owner-only), and any pre-existing looser log is tightened once on first write. The tighten flag is mutated only on the serial logging queue (no lock needed); the path is unchanged so documented readers (AGENTS.md, Sentry attachment, dev scripts) still work; first-write ordering preserves content.NSSavePaneland reveals it in Finder — no network, no Sentry.Tests
LoggerPermissionsTests(2/2: create-0600, tighten-existing with content preserved) +DiagnosticsExportTests(4/4: real offline writer, token/JWT/Bearer redaction, PII-bucketed snapshots, missing-log handled).bash test.sh→ 293 Rust + 131 Swift suites, all green (independently re-run by a verifier pass).Honest scope
NSSavePanelchooser + reveal-in-Finder is GUI-BLOCKED (system modal, not driven headlessly).Follow-up (noted, not in this PR)
The export's log-tail redaction is defensive best-effort on a log not expected to hold raw credentials (the app already masks secrets at emit time). It reliably masks JWT (
eyJ…),Bearer <tok>, andkey=/:<value>shapes, but misses: bare high-entropy tokens with no keyword (sk-proj-…), space-separated values, plainkey:(noapiprefix), andAuthorization: Basic <base64>. A fast follow can broaden value detection. No concrete unredacted-secret log line exists today.Changelog
desktop/macos/changelog/unreleased/20260705-diagnostics-export-log-hygiene.json