Skip to content

fix(frida): retain cancellation through final cleanup - #9963

Draft
morluto wants to merge 1 commit into
mainfrom
codex/audit-frida-final-cancellation
Draft

morluto wants to merge 1 commit into
mainfrom
codex/audit-frida-final-cancellation

Conversation

@morluto

@morluto morluto commented Nov 11, 2026

Copy link
Copy Markdown
Owner

Summary

A failed Frida session handshake, script load, or resume could lose the caller cancellation that arrived while the request or its final cleanup settled. The service now retains the exact caller reason as a secondary failure alongside the original physical error.

Problem and expected behavior

The one-shot load and resume failure paths closed the session and returned their primary error without reconciling the final caller signal. Failed start handshakes had the same omission. Callers need both the physical result and later cancellation, including when close also fails.

Change and scope

The service uses the existing shared cancellation owner for failed starts and consolidates load/resume failure cleanup into one private method. That method composes cleanup dependencies and attaches late caller cancellation after close. This is a lifecycle repair within existing Frida application authority, found during the repository audit.

Related PR #9934 changes capture-policy facts and touches separate assertions in the same test file. Its test patch applies cleanly on this branch.

Contract and boundary impact

The application service owns request finalization and the provider owns acquired Frida resources. Existing error projection exposes the retained secondary cancellation through the established error contract. Original failure identity, diagnostic facts, and cleanup dependencies stay available.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

The checked effects describe the existing Frida workflow. Validation used simulated providers.

Evidence and regression coverage

Eight assertions fail on base a6e02657 because secondary failures are empty. They cover both failed-start entry points, physical script-load failure before and during cleanup, and physical resume failure, including successful and failed close. After repair, all 37 service tests pass, including existing pending-start cleanup, completed script delivery, typed cancellation, retained source/message Evidence, and cleanup retry controls.

Validation performed

All commands used Node.js 24.22.1 in the isolated branch worktree.

  • npm run typecheck -- --incremental false passed.
  • Direct source Vitest for src/application/frida/FridaInstrumentationService.test.ts passed 37 tests.
  • Scoped oxfmt --check, oxlint, and git diff --check passed. Lint retained existing size, complexity, and parameter-count warnings.

Compatibility, safety, and release

The repair uses the established secondary-error contract. Real Frida attach, spawn, script execution, and remote cleanup were outside this verification. Package and installation contracts continue to apply.

Author readiness

Human owner:

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

This branch has not been deployed

No deployments
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