diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index c58db195f5..427189c0c9 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -450,7 +450,13 @@ Tool annotations describe effects and are hints, not authorization controls ([MCP ToolAnnotations](https://modelcontextprotocol.io/specification/2025-06-18/schema#toolannotations)). `readOnlyHint` includes session state: an analysis call that records additive Evidence is marked non-read-only even when it leaves the target unchanged. -`destructiveHint` describes possible data loss, not ordinary Evidence recording. +`destructiveHint` is true when a tool may discard data (overwriting or clearing +analyst metadata, replacing files, dropping session state), runs or drives code +REA does not own (`executesForeignCode`: process, LLDB, Electron, browser, and +native UI scenarios), or changes an attached Android device (`mutatesDevice`: +push, install, uninstall, start, and stop). Ordinary Evidence recording is not +destructive. The effect record in `binary_session` and the generated catalog +carries each flag separately. Effect metadata covers possible behavior across supported inputs: DMG inventory can launch `hdiutil` and create an owned temporary mount directory; extraction creates a fresh output directory on every call. Comparing supplied web captures diff --git a/src/contracts/toolEffects.test.ts b/src/contracts/toolEffects.test.ts new file mode 100644 index 0000000000..0fdcaf0498 --- /dev/null +++ b/src/contracts/toolEffects.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, it } from "vitest"; + +import { TOOL_CONTRACTS } from "./toolContracts.js"; +import { TOOL_EFFECTS, annotationsFromEffects } from "./toolEffects.js"; + +/** + * Descriptions that name running, launching, executing, starting, installing, + * or uninstalling something. A matching tool must declare foreign execution or + * device mutation unless it is listed below with the reason the wording does + * not describe that effect. + */ +const EXECUTION_WORDING = + /\b(?:run|runs|running|launch|launches|launching|execute|executes|executing|execution|start|starts|install|installs|uninstall|uninstalls)\b/iu; + +const EXECUTION_WORDING_EXCEPTIONS: Readonly> = { + inspect_native_load_image: "states that it does not execute the target", + resolve_native_call_targets: "states that it does not execute code", + inspect_pe_resources: "states that it does not execute the image", + inspect_binary_layout: + "parses the binary offline with operator-configured pwntools", + inspect_recorded_crash: "inspects a core file without launching its target", + inspect_macho: "runs host Mach-O inspection tools, not the target", + inspect_evm_interface: + "decodes bytecode in REA's bundled worker without executing it", + inspect_managed_artifact: "reads metadata without executing target code", + inspect_managed_members: "reads metadata without executing target code", + inspect_managed_native_boundaries: + "reads metadata without executing target code", + verify_managed_native_boundaries: "states it does not execute managed code", + project_managed_application_graph: "states it does not execute managed code", + inspect_android_package: "states it does not execute the APK", + search_android_classes: "states it does not execute the APK", + inspect_apktool_client: + "inspects the operator-selected apktool launcher; never installs it", + inspect_jeb_client: "talks to a caller-started JEB; never launches it", + open_jeb_project: "the engine analyzes the file; REA does not execute it", + inspect_adb_client: + "inspects the operator-selected adb client; never starts its server", + list_adb_devices: "may start the local adb server, which REA does not own", + inspect_firmware_regions: "states there is no target execution", + extract_firmware: "states that extracted code is never executed", + analyze_javascript_application: "reconstructs the graph without executing", + reconcile_javascript_runtime: "never invents execution claims", + observe_javascript_runtime: "never evaluates or resumes the target", + trace_application_feature: "never executes the application", + trace_javascript_semantics: "static reachability never claims execution", + compare_javascript_export_shapes: "states it does not execute JavaScript", + project_android_application_graph: "execution-free inventory", + project_apple_application_graph: "execution-free inventory", + trace_web_source_location: "states there is no application execution", + trace_web_module_imports: + "runs only REA's resolver stub in operator-configured Chromium", + export_web_scripts: "states there is no refetch or execution", + observe_web_execution: + "instruments an externally driven page; it does not run code itself", + recover_javascript_sources: "states there is no application code execution", + goto_address: "start names an object boundary, not a launch", + prev_address: "start names an object boundary, not a launch", + trace_call_path: "start names the traversal origin", + observe_native_ui: "observes an already-running app without launching it", + inspect_adb_package: "reports install times without changing the device", + list_adb_processes: "lists running processes without changing the device", + inspect_web_event_listeners: "states it does not evaluate JavaScript", +}; + +const declaresForeignAuthority = (name: string): boolean => { + const effects = TOOL_EFFECTS[name]; + return ( + effects !== undefined && + (effects.executesForeignCode || effects.mutatesDevice) + ); +}; + +describe("tool effects", () => { + it("audits every public tool exactly once", () => { + expect(Object.keys(TOOL_EFFECTS).sort()).toEqual( + TOOL_CONTRACTS.map(({ name }) => name).sort(), + ); + }); + + it("keeps execution and device wording aligned with declared effects", () => { + const undeclared = TOOL_CONTRACTS.filter( + ({ name, description }) => + EXECUTION_WORDING.test(description) && + !declaresForeignAuthority(name) && + EXECUTION_WORDING_EXCEPTIONS[name] === undefined, + ).map(({ name }) => name); + expect(undeclared).toEqual([]); + const staleExceptions = Object.keys(EXECUTION_WORDING_EXCEPTIONS).filter( + (name) => { + const contract = TOOL_CONTRACTS.find( + (candidate) => candidate.name === name, + ); + return ( + contract === undefined || + !EXECUTION_WORDING.test(contract.description) || + declaresForeignAuthority(name) + ); + }, + ); + expect(staleExceptions).toEqual([]); + }); + + it("marks foreign execution and device mutation as destructive", () => { + const destructive = TOOL_CONTRACTS.filter( + ({ effects }) => effects.executesForeignCode || effects.mutatesDevice, + ); + expect(destructive.map(({ name }) => name).sort()).toEqual([ + "capture_browser_scenario", + "capture_electron_scenario", + "capture_native_ui_scenario", + "capture_process_scenario", + "install_adb_package", + "observe_native_calls", + "push_adb_file", + "start_adb_activity", + "start_adb_app", + "stop_adb_app", + "uninstall_adb_package", + ]); + for (const contract of destructive) + expect(contract.annotations, contract.name).toMatchObject({ + readOnlyHint: false, + destructiveHint: true, + }); + }); + + it("derives destructiveHint from each destructive effect alone", () => { + const base = TOOL_EFFECTS["binary_session"]; + if (base === undefined) throw new Error("binary_session is unaudited"); + expect(annotationsFromEffects(base)).toEqual({ + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, + }); + for (const flag of [ + "mayDiscardData", + "executesForeignCode", + "mutatesDevice", + ] as const) + expect(annotationsFromEffects({ ...base, [flag]: true }), flag).toEqual({ + readOnlyHint: false, + destructiveHint: true, + idempotentHint: true, + openWorldHint: false, + }); + }); + + it("publishes the advertised annotations derived from canonical effects", () => { + for (const contract of TOOL_CONTRACTS) + expect(contract.annotations, contract.name).toEqual( + annotationsFromEffects(contract.effects), + ); + }); +}); diff --git a/src/contracts/toolEffects.ts b/src/contracts/toolEffects.ts index 4910e3c905..90edfaf18d 100644 --- a/src/contracts/toolEffects.ts +++ b/src/contracts/toolEffects.ts @@ -7,7 +7,17 @@ export interface ToolEffects { readonly launchesProcess: boolean; readonly accessesNetwork: boolean; readonly changesUiState: boolean; + /** Can overwrite, clear, or drop state that the caller cannot recover from REA. */ readonly mayDiscardData: boolean; + /** + * Runs or drives code REA does not own on the host: the analysis target, a + * caller-selected executable, or the application loaded into one. Running + * REA's own pinned helpers or operator-configured analysis engines does not + * count. + */ + readonly executesForeignCode: boolean; + /** Changes an attached Android device: packages, files, or running apps. */ + readonly mutatesDevice: boolean; readonly idempotent: boolean; } @@ -27,6 +37,8 @@ const effects = (overrides: Partial = {}): ToolEffects => ({ accessesNetwork: false, changesUiState: false, mayDiscardData: false, + executesForeignCode: false, + mutatesDevice: false, idempotent: true, ...overrides, }); @@ -178,6 +190,7 @@ export const TOOL_EFFECTS: Readonly> = { launchesProcess: true, writesFilesystem: true, accessesNetwork: true, + mutatesDevice: true, idempotent: false, }), capture_adb_screen: effects({ @@ -239,6 +252,7 @@ export const TOOL_EFFECTS: Readonly> = { mutatesSession: true, launchesProcess: true, accessesNetwork: true, + mutatesDevice: true, idempotent: false, }), uninstall_adb_package: effects({ @@ -246,6 +260,7 @@ export const TOOL_EFFECTS: Readonly> = { mutatesSession: true, launchesProcess: true, accessesNetwork: true, + mutatesDevice: true, idempotent: false, }), start_adb_app: effects({ @@ -253,6 +268,7 @@ export const TOOL_EFFECTS: Readonly> = { mutatesSession: true, launchesProcess: true, accessesNetwork: true, + mutatesDevice: true, idempotent: false, }), start_adb_activity: effects({ @@ -260,6 +276,7 @@ export const TOOL_EFFECTS: Readonly> = { mutatesSession: true, launchesProcess: true, accessesNetwork: true, + mutatesDevice: true, idempotent: false, }), stop_adb_app: effects({ @@ -267,6 +284,7 @@ export const TOOL_EFFECTS: Readonly> = { mutatesSession: true, launchesProcess: true, accessesNetwork: true, + mutatesDevice: true, idempotent: false, }), search_android_classes: effects({ @@ -294,6 +312,7 @@ export const TOOL_EFFECTS: Readonly> = { mutatesSession: true, launchesProcess: true, writesFilesystem: true, + mayDiscardData: true, }), inspect_native_load_image: evidence, inspect_native_dispatch_metadata: evidence, @@ -330,11 +349,31 @@ export const TOOL_EFFECTS: Readonly> = { resolve_containing_procedure: evidence, search_procedures: evidence, search_strings: evidence, - set_address_name: effects({ mutatesTarget: true, mutatesSession: true }), - set_addresses_names: effects({ mutatesTarget: true, mutatesSession: true }), - set_bookmark: effects({ mutatesTarget: true, mutatesSession: true }), - set_comment: effects({ mutatesTarget: true, mutatesSession: true }), - set_inline_comment: effects({ mutatesTarget: true, mutatesSession: true }), + set_address_name: effects({ + mutatesTarget: true, + mutatesSession: true, + mayDiscardData: true, + }), + set_addresses_names: effects({ + mutatesTarget: true, + mutatesSession: true, + mayDiscardData: true, + }), + set_bookmark: effects({ + mutatesTarget: true, + mutatesSession: true, + mayDiscardData: true, + }), + set_comment: effects({ + mutatesTarget: true, + mutatesSession: true, + mayDiscardData: true, + }), + set_inline_comment: effects({ + mutatesTarget: true, + mutatesSession: true, + mayDiscardData: true, + }), unset_bookmark: effects({ mutatesTarget: true, mutatesSession: true, @@ -365,6 +404,7 @@ export const TOOL_EFFECTS: Readonly> = { accessesNetwork: true, changesUiState: true, mayDiscardData: true, + executesForeignCode: true, idempotent: false, }), // The owned process can change files, show UI and use the network. @@ -374,6 +414,7 @@ export const TOOL_EFFECTS: Readonly> = { launchesProcess: true, accessesNetwork: true, changesUiState: true, + executesForeignCode: true, idempotent: false, }), inspect_macho: nativeEvidence, @@ -419,6 +460,7 @@ export const TOOL_EFFECTS: Readonly> = { accessesNetwork: true, changesUiState: true, mayDiscardData: true, + executesForeignCode: true, idempotent: false, }), list_electron_targets: browserEvidence, @@ -431,6 +473,7 @@ export const TOOL_EFFECTS: Readonly> = { accessesNetwork: true, changesUiState: true, mayDiscardData: true, + executesForeignCode: true, idempotent: false, }), list_javascript_runtime_targets: browserEvidence, @@ -479,6 +522,7 @@ export const TOOL_EFFECTS: Readonly> = { writesFilesystem: true, launchesProcess: true, accessesNetwork: true, + executesForeignCode: true, idempotent: false, }), compare_process_captures: sessionEvidence, @@ -506,9 +550,15 @@ export const annotationsFromEffects = ( value.launchesProcess || value.accessesNetwork || value.changesUiState || - value.mayDiscardData + value.mayDiscardData || + value.executesForeignCode || + value.mutatesDevice ), - destructiveHint: value.mayDiscardData, + // MCP defines destructive as possibly non-additive changes to the + // environment. Running foreign code or changing a device cannot be bounded + // to additive effects, so either one is destructive even without data loss. + destructiveHint: + value.mayDiscardData || value.executesForeignCode || value.mutatesDevice, idempotentHint: value.idempotent, openWorldHint: value.launchesProcess || value.accessesNetwork, }); diff --git a/src/contracts/toolOutputSchemaPrimitives.ts b/src/contracts/toolOutputSchemaPrimitives.ts index 1c4d449c89..51a53aa4f4 100644 --- a/src/contracts/toolOutputSchemaPrimitives.ts +++ b/src/contracts/toolOutputSchemaPrimitives.ts @@ -293,6 +293,8 @@ const toolAvailabilityFacts = { accessesNetwork: z.boolean(), changesUiState: z.boolean(), mayDiscardData: z.boolean(), + executesForeignCode: z.boolean(), + mutatesDevice: z.boolean(), idempotent: z.boolean(), }), annotations: z.object({ diff --git a/tests/boundary/mcp/toolResult.test.ts b/tests/boundary/mcp/toolResult.test.ts index a71ac46f63..90e1778f45 100644 --- a/tests/boundary/mcp/toolResult.test.ts +++ b/tests/boundary/mcp/toolResult.test.ts @@ -50,6 +50,8 @@ const contract: ToolContract = { accessesNetwork: false, changesUiState: false, mayDiscardData: false, + executesForeignCode: false, + mutatesDevice: false, idempotent: true, }, annotations: { diff --git a/tests/boundary/mcp/toolSchemaValidity.test.ts b/tests/boundary/mcp/toolSchemaValidity.test.ts index 6855f81885..adb5f83e99 100644 --- a/tests/boundary/mcp/toolSchemaValidity.test.ts +++ b/tests/boundary/mcp/toolSchemaValidity.test.ts @@ -73,7 +73,7 @@ function expectKnownAuthorityHints(tools: readonly ToolSchemas[]): void { }, annotate_native_function: { readOnlyHint: false, - destructiveHint: false, + destructiveHint: true, idempotentHint: true, openWorldHint: true, }, @@ -102,8 +102,22 @@ function expectKnownAuthorityHints(tools: readonly ToolSchemas[]): void { openWorldHint: false, }, capture_process_scenario: { + readOnlyHint: false, + destructiveHint: true, openWorldHint: true, }, + observe_native_calls: { + readOnlyHint: false, + destructiveHint: true, + }, + uninstall_adb_package: { + readOnlyHint: false, + destructiveHint: true, + }, + start_adb_app: { + readOnlyHint: false, + destructiveHint: true, + }, export_evidence_bundle: { readOnlyHint: false, destructiveHint: true,