ADR 0019: catch_unwind and panic = "abort" interaction
Status: Accepted Date: 2026-06-18
Context
The arbitraitor-analysis crate uses catch_unwind(AssertUnwindSafe(|| detector.analyze(ctx)))
to isolate detector panics — when a detector panics, the coordinator records
DetectorStatus::Error and continues running remaining detectors, then derives
an Incomplete verdict (fail-closed).
ADR 0001 establishes panic = "abort" for the release profile. Under
panic = "abort", catch_unwind is a no-op: a panic aborts the process
rather than unwinding the stack, so the catch_unwind closure never returns
Err and the DetectorStatus::Error path is never reached.
This means detector isolation via catch_unwind is functional in debug and
test builds only. In production (release) builds, a detector panic aborts the
entire process.
Decision
Accept the tension as an intentional two-tier defense:
-
Debug/test builds (
panic = "unwind"):catch_unwindprovides detector isolation. A panicking detector is caught, recorded asDetectorStatus::Error, and the analysis continues. The verdict isIncomplete. This enables robust testing of detector error handling. -
Release builds (
panic = "abort"): Process abort is the fail-closed mechanism. A detector panic kills the process, producing no verdict at all. The caller observes a non-zero exit and must treat the artifact as untrusted. This is strictly more conservative thanIncomplete.
The workspace clippy::panic = "deny" and unwrap_used = "deny" lints
minimize the sources of unintentional panics. Remaining panic sources (index
out of bounds in dependencies, stack overflow) are rare and result in process
abort in release — the safest possible outcome for a security boundary.
Consequences
DetectorStatus::Error(panic_message)and theIncompleteverdict for panics are dead code in release builds. They remain valuable for debug/test coverage and are not removed.DetectorStatus::Timeoutis similarly debug-only (see #79 for timeout enforcement design).- Future consumers of the analysis pipeline must distinguish “no verdict due
to process abort” from an explicit
Incompleteverdict. The receipt schema should account for this in a future revision. - The
AssertUnwindSafewrapper is sound:analyze(&self, &AnalysisContext)holds only immutable references, so no mutation crosses the unwind boundary.
Alternatives Considered
-
panic = "unwind"in release: Rejected by ADR 0001 for binary size, compile time, and defense-in-depth reasons. Process abort is the preferred fail-closed mechanism for a security boundary. -
Spawn each detector in a separate process: Would provide isolation even under
panic = "abort", but introduces IPC complexity, serialization overhead, and non-determinism — unacceptable for the MVP. -
Remove
catch_unwindentirely: Would simplify the code but lose valuable debug-time detector isolation and error reporting.