Validate replacement block signatures

[steipete]

Summary

• validate replacement block Objective-C type encodings before creating an IMP
• reject mismatched return types, argument counts, and argument types with a typed InterposeError
• cover class and object hooks, and document that callers still type the original implementation signature

Root cause

InterposeKit trusted the generic HookSignature supplied by the caller. The Objective-C runtime therefore accepted blocks with a calling convention that did not match the target method.

On unmodified master, an object hook replaced a method returning an object with a block returning Int. Hook construction succeeded, then invoking the method on macOS 26.5 with Xcode 26.6 terminated XCTest with signal 11.

The fix reads the Objective-C signature embedded in the block ABI and compares it with the target Method before calling imp_implementationWithBlock.

Verification

• swift test
• swift test -c release
• macOS tests with YAMS_DEFAULT_ENCODING=UTF16
• macOS tests with YAMS_DEFAULT_ENCODING=UTF8
• iOS 26.5 simulator tests
• watchOS 26.5 simulator build
• optimized release-linking proof
• Objective-C class-load watcher and legacy dyld fallback proofs
• $autoreview: clean

Closes #27

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed June 11, 2026, 7:35 AM ET / 11:35 UTC.

Summary
The PR adds Objective-C block ABI signature extraction and validates replacement blocks against target method type encodings before creating IMPs, with class/object-hook tests and README/changelog updates.

Reproducibility: yes. Source inspection shows current master calls imp_implementationWithBlock without comparing block and method encodings, matching the PR's crash scenario; I did not execute XCTest because this review is read-only.

Review metrics: 1 noteworthy metric.

• Changed surface: 6 production files, 2 test files, 2 docs/release-note files. The patch crosses Swift hook construction, Objective-C runtime parsing, public errors, and user-facing documentation.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P2] Confirm release/versioning treatment for the new fail-fast error path before merge.

Risk before merge

• [P1] Existing callers with mismatched replacement block signatures that previously constructed hooks will now receive InterposeError before IMP creation; that fail-fast behavior is intentional but upgrade-visible.
• [P1] Adding a public InterposeError case can affect downstream source that exhaustively switches over the enum, so release/versioning expectations should be deliberate.

Maintainer options:

1. Accept fail-fast validation (recommended)
   Merge after maintainer confirmation that mismatched-signature hooks should fail during construction and the new public error case is acceptable for the next release.
2. Add compatibility guardrails
   If downstream source compatibility is a concern, add release/versioning notes or narrower compatibility documentation before merge.
3. Pause for ABI review
   Pause if maintainers want a broader sweep for valid Swift/Objective-C signatures that might be falsely rejected by the type-encoding comparison.

Next step before merge

• Owner-authored PR with compatibility-sensitive runtime behavior should stay in normal maintainer review rather than cleanup or automated repair.

Security
Cleared: Cleared: the diff adds in-process block metadata parsing and no dependency, workflow, secret, or package-resolution changes.

Review details

Best possible solution:

Land the signature validation once maintainers are satisfied the intentional fail-fast behavior and public error case are acceptable for the next release.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows current master calls imp_implementationWithBlock without comparing block and method encodings, matching the PR's crash scenario; I did not execute XCTest because this review is read-only.

Is this the best way to solve the issue?

Yes. The proposed fix is narrow: read the block ABI signature, compare return and argument encodings against the Method, and reject before creating an IMP; the remaining judgment is compatibility/versioning.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against a7e1b57bedf5.

Label changes

Label changes:

• add P2: This is a normal-priority crash-prevention bug fix with limited blast radius around mis-typed hook definitions.
• add merge-risk: 🚨 compatibility: The PR intentionally changes upgrade behavior for mismatched hooks and adds a public error enum case.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The real-behavior proof gate does not apply to an OWNER-authored PR; the body still lists broad local and simulator verification.

Label justifications:

• P2: This is a normal-priority crash-prevention bug fix with limited blast radius around mis-typed hook definitions.
• merge-risk: 🚨 compatibility: The PR intentionally changes upgrade behavior for mismatched hooks and adds a public error enum case.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The real-behavior proof gate does not apply to an OWNER-authored PR; the body still lists broad local and simulator verification.

Evidence reviewed

What I checked:

• Current main trusts replacement blocks: On current master, class and object hooks create replacement IMPs directly from the supplied block, and README documents that missing type checking can crash at runtime. (Sources/InterposeKit/ClassHook.swift:12, a7e1b57bedf5)
• PR validates before IMP creation: The PR adds validateImplementationBlock, calls IKTBlockSignatureMatchesMethod, and throws InterposeError.incompatibleHookSignature before imp_implementationWithBlock runs. (Sources/InterposeKit/AnyHook.swift:73, 3285655270e0)
• Block ABI comparison is centralized: The C helper reads the block type encoding, builds NSMethodSignature values, compares return type, self, and method arguments, and rejects mismatches. (Sources/SuperBuilder/src/ITKSuperBuilder.m:70, 3285655270e0)
• Regression coverage exists in the PR: The PR adds class-hook and object-hook tests for mismatched return and argument signatures expecting the new typed error. (Tests/InterposeKitTests/ObjectInterposeTests.swift:7, 3285655270e0)
• History points to the same owner area: Blame shows the direct imp_implementationWithBlock hook constructors were introduced in the original object-hook work, and recent log history shows the same area was actively maintained on current master. (Sources/InterposeKit/ClassHook.swift:12, 9e387e1e8936)
• Cleanup close is blocked by item ownership: The provided GitHub context marks this open pull request as authorAssociation OWNER, so ClawSweeper should keep it open for maintainer handling.

Likely related people:

• steipete: Git blame and log history show the class/object hook constructors, direct block-to-IMP path, and recent object-hook safety changes are primarily from this contributor. (role: introduced behavior and recent area contributor; confidence: high; commits: 9e387e1e8936, 7a9367e1743a, 7413a066a3d2; files: Sources/InterposeKit/ClassHook.swift, Sources/InterposeKit/ObjectHook.swift, Sources/SuperBuilder/src/ITKSuperBuilder.m)
• ishutinvv: Git history for ITKSuperBuilder.m includes adjacent Objective-C runtime/trampoline work in the same low-level support file. (role: adjacent runtime contributor; confidence: medium; commits: c3a2ef45274f; files: Sources/SuperBuilder/src/ITKSuperBuilder.m)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.