[audit] Deep audit — missing tests, concurrency issues, error handling #2

New issue

Open

opened 2026-03-22 16:37:31 +00:00 by Virgil · 4 comments

Virgil commented

2026-03-22 16:37:31 +00:00

Member

Prior AX sweep found:

3 HIGH (Close blocks indefinitely, CloseTab kills browser, Angular wait not awaited)
6 MEDIUM (concurrent handler removal, unbounded buffer, selector injection)
2 LOW (resource leak, missing SPDX headers)

This audit should go deeper: find missing test coverage, race conditions, error handling gaps, and any additional security concerns not caught in the first pass.

Prior AX sweep found: - 3 HIGH (Close blocks indefinitely, CloseTab kills browser, Angular wait not awaited) - 6 MEDIUM (concurrent handler removal, unbounded buffer, selector injection) - 2 LOW (resource leak, missing SPDX headers) This audit should go deeper: find missing test coverage, race conditions, error handling gaps, and any additional security concerns not caught in the first pass.

Virgil commented

2026-03-22 16:46:53 +00:00

Author

Member

Spark Audit Findings (24 total)

HIGH (3)

CDPClient.Close blocks forever (cdp.go:156)
CloseTab kills entire browser (cdp.go:339)
Angular waitForZoneStability not awaited (angular.go:173)

MEDIUM (8)

Resource leak in New (webview.go:134)
readLoop busy-spins (cdp.go:223)
Handler removal race (console.go:194)
Shared params data race (cdp.go:260)
SSRF via raw http.Get (cdp.go:80)
JS injection via formatJSValue (angular.go:627)
16.1% test coverage
Missing SPDX headers

LOW (2)

No watcher unsubscribe (console.go:33)

## Spark Audit Findings (24 total) ### HIGH (3) 1. CDPClient.Close blocks forever (cdp.go:156) 2. CloseTab kills entire browser (cdp.go:339) 3. Angular waitForZoneStability not awaited (angular.go:173) ### MEDIUM (8) 4. Resource leak in New (webview.go:134) 5. readLoop busy-spins (cdp.go:223) 6. Handler removal race (console.go:194) 7. Shared params data race (cdp.go:260) 8. SSRF via raw http.Get (cdp.go:80) 9. JS injection via formatJSValue (angular.go:627) 10. 16.1% test coverage 11. Missing SPDX headers ### LOW (2) 12. No watcher unsubscribe (console.go:33)

Virgil referenced this issue

2026-03-23 07:34:45 +00:00

[agent/codex] Fix ALL findings from issue #2. Read CLAUDE.md first. CDPCli... #3

Virgil referenced this issue from a commit

2026-03-23 07:34:47 +00:00

Merge pull request '[agent/codex] Fix ALL findings from issue #2. Read CLAUDE.md first. CDPCli...' (#3) from agent/deep-audit-per-issue--2--read-claude-md into dev

Virgil commented

2026-03-23 07:34:57 +00:00

Author

Member

Fix Applied

Commit dff3d57: fix(cdp): resolve issue 2 audit findings

CDPClient.Close blocking fixed
CloseTab no longer kills browser
Angular waitForZoneStability properly awaited
Resource leak in New fixed
readLoop busy-spin fixed
Handler removal race fixed
Shared params data race fixed
http.Get replaced with proper client
JS injection via formatJSValue fixed
673-line audit_issue2_test.go added
1,248 additions across 7 files

## Fix Applied Commit dff3d57: fix(cdp): resolve issue 2 audit findings - CDPClient.Close blocking fixed - CloseTab no longer kills browser - Angular waitForZoneStability properly awaited - Resource leak in New fixed - readLoop busy-spin fixed - Handler removal race fixed - Shared params data race fixed - http.Get replaced with proper client - JS injection via formatJSValue fixed - 673-line audit_issue2_test.go added - 1,248 additions across 7 files

Virgil commented

2026-03-23 08:54:25 +00:00

Author

Member