diff --git a/src/config.test.ts b/src/config.test.ts index dd228e4a6..6643fbdd1 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { parseConfig } from "./config.js"; +import { projectAnalysisError } from "./domain/analysisErrorProjection.js"; describe("runtime configuration", () => { it("allows target-free startup and applies runtime defaults", () => { @@ -155,3 +156,45 @@ describe("runtime collection configuration", () => { }, ); }); + +describe("configuration failure diagnostics", () => { + it.each([ + ["GHIDRA_INSTALL_DIR", "./fixture"], + ["JAVA_HOME", "./jdk"], + ["REA_GHIDRA_NATIVEAOT_JAR", "./missing.jar"], + ])( + "names the rejected %s and its absolute-path constraint", + (setting, value) => { + const result = parseConfig({ [setting]: value }); + expect(result.ok).toBe(false); + if (result.ok) return; + const constraint = `${setting} must be absolute`; + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "configuration_invalid", + message: `REA configuration is invalid: ${constraint}.`, + remediation: { action: expect.stringContaining(`Correct ${setting} `) }, + details: { settings: [{ setting, constraint }] }, + }); + // The rejected value itself is not echoed. + expect(JSON.stringify(projectAnalysisError(result.error))).not.toContain( + value, + ); + }, + ); + + it("names settings rejected by their own parsers", () => { + const result = parseConfig({ HOPPER_LOADER_ARGS_JSON: "not-json" }); + expect(result.ok).toBe(false); + if (!result.ok) + expect(projectAnalysisError(result.error)).toMatchObject({ + details: { + settings: [ + { + setting: "HOPPER_LOADER_ARGS_JSON", + constraint: "HOPPER_LOADER_ARGS_JSON must be valid JSON", + }, + ], + }, + }); + }); +}); diff --git a/src/config/environment.ts b/src/config/environment.ts index fe7e1245e..f743de576 100644 --- a/src/config/environment.ts +++ b/src/config/environment.ts @@ -53,9 +53,15 @@ export const parseEnvironment = ( ): Result => { const parsed = environmentSchema.safeParse(environment); if (!parsed.success) { + // Zod messages name the constraint, never the rejected value. + const settings = parsed.error.issues.map((issue) => ({ + setting: String(issue.path[0] ?? "environment"), + constraint: issue.message, + })); return err( new ConfigurationError("Invalid REA environment configuration", { cause: parsed.error, + settings, }), ); } diff --git a/src/config/mcpResponseBudget.ts b/src/config/mcpResponseBudget.ts index 01d69db97..27126bf0c 100644 --- a/src/config/mcpResponseBudget.ts +++ b/src/config/mcpResponseBudget.ts @@ -15,6 +15,8 @@ export const mcpResponseBudgetSchema = z .transform(Number) .pipe(z.number().int().safe().min(MINIMUM_RESPONSE_BYTES)); +const budgetConstraint = `REA_MCP_MAX_RESPONSE_BYTES must be a decimal safe integer at least ${String(MINIMUM_RESPONSE_BYTES)} bytes, the default stdio receive-buffer size.`; + /** Parse only the MCP-specific budget without loading unrelated provider settings. */ export const parseMcpResponseBudget = ( value: string | undefined, @@ -24,8 +26,13 @@ export const parseMcpResponseBudget = ( return parsed.success ? ok(parsed.data) : err( - new ConfigurationError( - `REA_MCP_MAX_RESPONSE_BYTES must be a decimal safe integer at least ${String(MINIMUM_RESPONSE_BYTES)} bytes, the default stdio receive-buffer size.`, - ), + new ConfigurationError(budgetConstraint, { + settings: [ + { + setting: "REA_MCP_MAX_RESPONSE_BYTES", + constraint: budgetConstraint, + }, + ], + }), ); }; diff --git a/src/config/parsers.ts b/src/config/parsers.ts index 41637d513..344b31eea 100644 --- a/src/config/parsers.ts +++ b/src/config/parsers.ts @@ -14,10 +14,21 @@ export const parseStringArray = ( : err( new ConfigurationError(`${name} must encode an array of strings`, { cause: parsed.error, + settings: [ + { + setting: name, + constraint: `${name} must encode an array of strings`, + }, + ], }), ); } catch (cause: unknown) { - return err(new ConfigurationError(`${name} must be valid JSON`, { cause })); + return err( + new ConfigurationError(`${name} must be valid JSON`, { + cause, + settings: [{ setting: name, constraint: `${name} must be valid JSON` }], + }), + ); } }; @@ -32,6 +43,9 @@ export const parseLoaderArgs = ( return err( new ConfigurationError("HOPPER_LOADER_ARGS_JSON must be valid JSON", { cause, + settings: [ + loaderArgsIssue("HOPPER_LOADER_ARGS_JSON must be valid JSON"), + ], }), ); } @@ -41,7 +55,19 @@ export const parseLoaderArgs = ( : err( new ConfigurationError( "HOPPER_LOADER_ARGS_JSON must encode an array of strings", - { cause: parsed.error }, + { + cause: parsed.error, + settings: [ + loaderArgsIssue( + "HOPPER_LOADER_ARGS_JSON must encode an array of strings", + ), + ], + }, ), ); }; + +const loaderArgsIssue = (constraint: string) => ({ + setting: "HOPPER_LOADER_ARGS_JSON", + constraint, +}); diff --git a/src/domain/analysisErrorPresentation.ts b/src/domain/analysisErrorPresentation.ts index cc3dc7e40..7676b2b39 100644 --- a/src/domain/analysisErrorPresentation.ts +++ b/src/domain/analysisErrorPresentation.ts @@ -78,6 +78,8 @@ export const analysisErrorRemediationAction = ( return "Check the current process's read access to the selected path. Retry with a readable local file."; if (error instanceof AnalysisArtifactChangedError) return "Wait until the selected file is stable. For an active binary session, reopen the target with open_binary before retrying so REA acquires its current identity; for a CLI command or target-free tool, rerun the operation."; + if (error instanceof ConfigurationError && error.settings.length > 0) + return `Correct ${[...new Set(error.settings.map(({ setting }) => setting))].join(", ")} in the environment that starts REA (your shell or the MCP client's registration), then rerun.`; if (error instanceof AnalysisInputError) return "Correct the listed arguments and retry."; if (error instanceof UnknownRegistryError && error.reason === "not-found") @@ -189,7 +191,15 @@ export const analysisErrorUserMessage = (error: AnalysisError): string => { if (error instanceof UnknownRegistryError) return "Evidence state changed before the update completed. Refresh the current state and try again."; if (error instanceof ConfigurationError) - return "REA configuration is invalid. Run `rea doctor` and fix the reported setting."; + return error.settings.length === 0 + ? "REA configuration is invalid. Run `rea doctor` and fix the reported setting." + : `REA configuration is invalid: ${error.settings + .map(({ setting, constraint }) => + constraint.includes(setting) + ? constraint.replace(/\.$/u, "") + : `${setting}: ${constraint}`, + ) + .join("; ")}.`; if (error instanceof NoBinaryOpenError) return error.message; if (error instanceof BinaryTargetError) return error.constraint === "directory_requires_file" diff --git a/src/domain/analysisErrorProjection.ts b/src/domain/analysisErrorProjection.ts index 66e5c77e9..7fd0241ca 100644 --- a/src/domain/analysisErrorProjection.ts +++ b/src/domain/analysisErrorProjection.ts @@ -18,7 +18,10 @@ import { AnalysisResourceConstraintError, } from "./analysisErrorCore.js"; import { ArtifactOperationError } from "./artifactOperationError.js"; -import { BinaryTargetError } from "./configurationErrors.js"; +import { + BinaryTargetError, + ConfigurationError, +} from "./configurationErrors.js"; import { BrowserObservationError } from "./browserObservationError.js"; import { EvidenceFileError, @@ -482,6 +485,13 @@ const lifecycleErrorDetails = ( }; return Object.keys(details).length === 0 ? undefined : details; } + if (error instanceof ConfigurationError && error.settings.length > 0) + return { + settings: error.settings.map(({ setting, constraint }) => ({ + setting, + constraint, + })), + }; if (error instanceof BinaryTargetError) return { path: error.path, diff --git a/src/domain/configurationErrors.ts b/src/domain/configurationErrors.ts index c7c07f75e..5820db7ff 100644 --- a/src/domain/configurationErrors.ts +++ b/src/domain/configurationErrors.ts @@ -3,9 +3,27 @@ import { type AnalysisErrorOptions, } from "./analysisErrorBase.js"; +/** One rejected configuration setting and the constraint it failed. */ +export interface ConfigurationSettingIssue { + readonly setting: string; + readonly constraint: string; +} + +/** Rejected settings retained for diagnostics; their values are not. */ +export interface ConfigurationErrorOptions extends AnalysisErrorOptions { + readonly settings?: readonly ConfigurationSettingIssue[]; +} + /** Runtime configuration could not be parsed safely. */ export class ConfigurationError extends AnalysisError { readonly _tag = "ConfigurationError"; + readonly settings: readonly ConfigurationSettingIssue[]; + constructor(message: string, options?: ConfigurationErrorOptions) { + super(message, options); + this.settings = (options?.settings ?? []).map( + ({ setting, constraint }) => ({ setting, constraint }), + ); + } } /** No app or binary session exists for an analysis request. */ diff --git a/src/ida/IdaConfiguration.ts b/src/ida/IdaConfiguration.ts index 051ee8d4d..26084b734 100644 --- a/src/ida/IdaConfiguration.ts +++ b/src/ida/IdaConfiguration.ts @@ -50,7 +50,7 @@ export const readIdaConfiguration = ( try { if (statSync(path).size > 65_536) return err( - new ConfigurationError( + idaConfigurationError( "REA_IDA_MCP_CONFIG exceeds the 64 KiB registration size.", ), ); @@ -64,17 +64,27 @@ export const readIdaConfiguration = ( return parsed.success ? ok(parsed.data) : err( - new ConfigurationError( + idaConfigurationError( "REA_IDA_MCP_CONFIG must contain an IDA MCP command/args or local url registration; see docs/ida-provider.md.", - { cause: parsed.error }, + parsed.error, ), ); } catch (cause: unknown) { return err( - new ConfigurationError( + idaConfigurationError( `Cannot read IDA MCP registration ${path}; provide a readable JSON configuration.`, - { cause }, + cause, ), ); } }; + +/** A REA_IDA_MCP_CONFIG rejection that names the setting it concerns. */ +export const idaConfigurationError = ( + constraint: string, + cause?: unknown, +): ConfigurationError => + new ConfigurationError(constraint, { + ...(cause === undefined ? {} : { cause }), + settings: [{ setting: "REA_IDA_MCP_CONFIG", constraint }], + }); diff --git a/src/ida/IdaProvider.ts b/src/ida/IdaProvider.ts index 6f195cb61..ff8a46fbb 100644 --- a/src/ida/IdaProvider.ts +++ b/src/ida/IdaProvider.ts @@ -11,13 +11,14 @@ import type { ProviderTargetSupport, } from "../application/AnalysisProvider.js"; import type { AppConfig } from "../config.js"; -import { ConfigurationError } from "../domain/configurationErrors.js"; +import type { ConfigurationError } from "../domain/configurationErrors.js"; import { createHash } from "node:crypto"; import canonicalize from "canonicalize"; import type { Result } from "../domain/result.js"; import { readIdaConfiguration, type IdaConfiguration, + idaConfigurationError, } from "./IdaConfiguration.js"; import { createIdaMcpConnection, @@ -165,7 +166,7 @@ export class IdaProvider implements AnalysisProviderCandidate { const readIdaConfigurationNotConfigured = () => err( - new ConfigurationError( + idaConfigurationError( "Set REA_IDA_MCP_CONFIG to an upstream IDA MCP JSON registration; see docs/ida-provider.md.", ), );