Skip to content

Fix 2026-09-11 codebase audit findings - #334

Open
arzafran wants to merge 66 commits into
mainfrom
fix/audit-remediation-20260911
Open

arzafran wants to merge 66 commits into
mainfrom
fix/audit-remediation-20260911

Conversation

@arzafran

@arzafran arzafran commented Sep 11, 2026

Copy link
Copy Markdown
Member

What this does

Fixes all 42 findings from the 2026-09-11 codebase audit (8 high, 31 medium, 3 low) so a fresh build can be cut. The highest-impact effects for users and agents:

  • Browser context menus no longer leak cookies from other profiles, and switching profiles keeps the configured proxy.
  • Clipboard reads require real consent; recipe prefill no longer submits before the promised Return.
  • Turning scrollback persistence off actually stops recording, and failed session writes are no longer reported as saved.
  • Applying a layout cannot force-close a live terminal; malformed selectors no longer widen destructive commands.
  • CLI default targeting can no longer type into another pane; alternate clients must authenticate; MCP honors its wait deadline.
  • Browser dialog accept/dismiss now answers the real native dialog instead of a fake queue.
  • Review panels no longer show stale or falsely-clean diffs and keep refreshing after being moved.
  • Update checks that time out show an error instead of claiming the app is current; release notes link to the feed's real URL.
  • Mermaid fences can no longer inject script into the diagram renderer.

Review order

  1. Security and persistence: H1 to H8 commits (cookies, clipboard, scrollback WAL, layout, socket peer, autosave, escrow FD).
  2. CLI and MCP transport: M1 to M4, H4.
  3. Browser automation: M5, M6, M7, M11.
  4. Workspace, review, sidebar: M12 to M15, M28, M31.
  5. Markdown, updates, transient UI: M16, M21 to M27, L2.
  6. Verification machinery and CI: M19, M20, M29, M30, ci.yml lifecycle phase.
  7. Docs and localization: L1, L3.

Test plan

  • CI unit and socket suites green on this PR head (run 34694747866 at fbe35a0).
  • macOS 26 focused notification regression green on dispatch run 34694763742 (all six jobs green). The earlier failures were XCUIApplication.state reporting foreground on the virtual display while the app never became active; the test now judges focus by the workspace's frontmost application.
  • Typing-lag check passed on dispatch run 34574619008 (churn p95 12.81 ms, cap 35 ms). The two earlier failures, including one on unchanged main, were runner variance.
  • Manual: open a browser surface, trigger confirm()/prompt(), answer via programa browser dialog accept "" and confirm the page receives an empty string.
  • Manual: move a review panel to another window and verify it still refreshes when the source agent goes idle.

Notes

  • Regression tests were landed red-then-green on separate verify/* branches during the Codex run (see the verify/audit-* branches).
  • Commit d3ef254 briefly added docs/audits and is undone by 9e21792; squash-merge or a history rewrite before landing removes it from main entirely.
  • The last commit (fbe35a0) is unsigned at the user's request while 1Password was locked.
  • The scrollback lifecycle fixture now launches with argument-domain preferences; SessionRestorePolicy ignores single-dash -key value pairs so restore still runs (5171c0a).

@arzafran
arzafran marked this pull request as ready for review September 12, 2026 13:48

@darkestdarky-bot darkestdarky-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff applies targeted fixes for audit findings: CI gains notification UI regression and scrollback privacy tests; MCP socket bridge now respects per-method timeouts and retries on EAGAIN; CLI argument parsing protects flag values from being stripped as trailing flags, and dialog accept preserves verbatim text; hook confirmation logic is corrected. No secrets, injection, data-loss paths, or type-safety escapes were observed in the visible hunks.

Review coverage: 500/11735 diff lines supplied. Partial input (truncated: diff, standards). Inline comments are limited to fully visible, valid right-side hunks. Reviewed commit: fbe35a0a730255c0e7565c5b277a8c5c94ce5db8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant