Skip to content

Fix: test harness rewritten around a connection-owning HAP client, removing the axios dependency.#1122

Merged
hjdhjd merged 2 commits into
latestfrom
fix/test-harness-arrival-await
Jul 15, 2026
Merged

Fix: test harness rewritten around a connection-owning HAP client, removing the axios dependency.#1122
hjdhjd merged 2 commits into
latestfrom
fix/test-harness-arrival-await

Conversation

@hjdhjd

@hjdhjd hjdhjd commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Motivation

The free-socket data guard introduced by the CVE-2026-48931 fix (Node 22.23 / 24.17 / 26.3) destroys idle keep-alive agent sockets on any unsolicited read, which broke the 26 encrypted-session and event-notification tests. 17f19f1 restored green by neutralizing the guard - harvesting Node's internal onStreamRead from a probe socket and overwriting socket._handle.onread on the stolen agent socket.

That workaround carries some structural risk I'd prefer to remove:

  • It depends on two layers of undocumented Node internals (agent.freeSockets and socket._handle.onread), and the guard itself arriving via a security backport to stable lines shows this class of dependency can now break on minor releases without warning.
  • It disables a CVE mitigation on the socket, putting the harness in a state no real Node HTTP client will be in again.
  • The underlying constraint it works around ("we cannot use a fresh socket, because the HAP encryption keys are bound to this exact server-side connection") only holds because the session is established through the agent and the socket is swapped afterwards.

What this PR does

HAPHTTPClient now owns a raw net.Socket for the connection's entire lifetime - the client-side mirror of the server's HAPConnection, and the same shape as a real HomeKit controller. Pair-setup, pair-verify, the encrypted session, and server-pushed EVENT/1.0 notifications all ride one owned connection, so the key-binding constraint disappears rather than needing a workaround. One client instance = one TCP connection = one server-side HAPConnection, which also makes the connection-scoped pairing-state tests (M1 reset prevention, crafted mid-handshake requests) explicit instead of relying on agent pool reuse.

Along the way:

  • axios removed (devDependency, ~9 transitive packages pruned). PairSetupClient / PairVerifyClient ride the owned client; plain request/response call sites use it too.
  • Fixed-delay waits replaced with arrival-based reads. writeHTTPRequest awaits actual response bytes with a generous deadline instead of PromiseTimeout(20), and responses are parsed incrementally across TCP segments (no single-segment assumption). Bytes trailing a parsed response (e.g. a coalesced EVENT message) are retained for the next read.
  • Event-timing assertions are anchored to the coalescing window. The 250ms literal is now HAPConnection.EVENT_COALESCING_DELAY (the only production-file change, behavior-preserving), and immediate-delivery assertions bound their waits below it - so a regression from immediate to timer-based delivery fails loudly instead of passing with identical bytes.
  • Client-side teardown is observed directly. The unpair test asserts socket close events on the owned connections instead of reading (agent as any).totalSocketCount, and every test's clients are torn down through one afterEach path.
  • Status assertions preserved. axios rejected on non-2xx implicitly; the migrated call sites assert status codes explicitly, including per-hop 200 OK in the pairing orchestrators.

Test-only change apart from the named constant: tsconfig.json excludes src/test-utils and *.spec.ts from the build, so the published package is unaffected.

Verification

  • Full suite 817/817 on Node 22.23.1, 24.18.0, and 26.4.0 (all three carry the free-socket guard), plus repeated runs of the two affected suites to check for timing flakes.
  • npm run build, npm run lint, and typedoc --emit none all clean.
  • restoreFreeSocketDataGuard and all agent-internals access removed; repo-wide grep for axios returns only the historical CHANGELOG entry.

@coveralls

coveralls commented Jul 12, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 29264740127

Coverage increased (+0.04%) to 66.203%

Details

  • Coverage increased (+0.04%) from the base build.
  • Patch coverage: 15 uncovered changes across 1 file (133 of 148 lines covered, 89.86%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
src/test-utils/HAPHTTPClient.ts 129 114 88.37%
Total (4 files) 148 133 89.86%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 9553
Covered Lines: 6646
Line Coverage: 69.57%
Relevant Branches: 3395
Covered Branches: 1926
Branch Coverage: 56.73%
Branches in Coverage %: Yes
Coverage Strength: 223.24 hits per line

💛 - Coveralls

@hjdhjd
hjdhjd requested review from NorthernMan54 and bwp91 July 12, 2026 21:52
NorthernMan54
NorthernMan54 previously approved these changes Jul 13, 2026

Copilot AI 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.

Pull request overview

This PR rewrites the test harness HTTP client to own a single TCP connection for its full lifetime, aligning the tests with how HAP connections and encryption/session keys are actually bound server-side, and removing reliance on Node’s http.Agent free-socket behavior (and its security guard). It also removes the axios dependency and updates affected specs to use the new connection-owning client.

Changes:

  • Introduce/expand HAPHTTPClient as a raw-socket, connection-owning client with incremental HTTP response parsing and event-friendly buffering.
  • Migrate pair-setup / pair-verify test drivers and server/eventedhttp specs from axios+Agent to the owned-connection client, with explicit status assertions.
  • Remove axios from devDependencies and lockfile; add HAPConnection.EVENT_COALESCING_DELAY constant and use it in event coalescing logic/tests.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/test-utils/PairVerifyClient.ts Switch pair-verify handshake driver from axios/Agent to HAPHTTPClient and add explicit status assertions.
src/test-utils/PairSetupClient.ts Switch SRP pair-setup driver from axios/Agent to HAPHTTPClient and add explicit status assertions.
src/test-utils/HAPHTTPClient.ts Rework test HTTP client to own a raw socket, add arrival-based reads, incremental parsing, and event-friendly buffering.
src/lib/util/eventedhttp.ts Introduce EVENT_COALESCING_DELAY constant and use it for the event flush timer.
src/lib/util/eventedhttp.spec.ts Migrate tests to HAPHTTPClient, replace fixed sleeps with arrival-based reads, and anchor timing to coalescing delay.
src/lib/HAPServer.spec.ts Replace axios-based requests with HAPHTTPClient across pairing, identify, auth, and error-path tests.
package.json Remove axios devDependency.
package-lock.json Remove axios and transitive dependencies from lockfile.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/test-utils/HAPHTTPClient.ts
Comment thread src/test-utils/HAPHTTPClient.ts
Comment thread src/lib/util/eventedhttp.spec.ts
… rejection, and portable loopback dialing in the test harness.
@hjdhjd
hjdhjd merged commit 5d75240 into latest Jul 15, 2026
14 checks passed
@hjdhjd
hjdhjd deleted the fix/test-harness-arrival-await branch July 15, 2026 03:58
@bwp91 bwp91 mentioned this pull request Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants