Skip to content

Tool validation errors echo the rejected input value (input_value) — add a masking option or a public validation-error hook #3572

Description

@panandr

What happens

When a tool call's arguments fail pydantic validation, Tool.run wraps the ValidationError into a ToolError whose message includes the pydantic rendering — and str(ValidationError) carries the offending value:

Error executing tool <name>: 1 validation error for <model>
<field>
  Input should be a valid string [type=string_type, input_value=12345, input_type=int]

That message is sent to the client as an isError result, so the rejected value is echoed back to the caller (and into the model's context).

Why it matters

Servers that handle sensitive input (PII/PHI, credentials, member identifiers) must not let a validation failure repeat the value; the error should describe the rule, not the data. We hit this building a healthcare-platform MCP server whose tool inputs can be PHI.

Workaround we use today (reaches private API)

We replace the SDK-generated arguments model with a subclass that raises MCPError(-32602, "<field>: <rule>") instead of the default ToolError:

tool = server._tool_manager.add_tool(fn, ...)
tool.fn_metadata.arg_model = PhiSafeArgModel(tool.fn_metadata.arg_model)

This depends on _tool_manager and fn_metadata.arg_model (both private) and on validation continuing to flow through model_validate, so it is fragile across minor releases.

Asks (any one would remove the private-API dependency)

  1. An opt-in setting to omit input values from tool validation errors (e.g. a mask_error_details-style flag), or
  2. A public/supported hook to customize argument-validation failures (or documenting arg_model as an extensible point), or
  3. Excluding input_value from the argument-validation error text by default.

Version: mcp 2.2.0 (Python). Happy to open a PR if you can point at the preferred shape.

Activity

  1. sattyamjjain commented on Sep 24, 2026

    @sattyamjjain

    +1 for option 3 (drop input_value by default), with option 2 as the escape hatch.

    Pydantic already has both switches, so the change may be small:

    • e.errors(include_input=False) returns the error list without the rejected values, so the ToolError message can be built from that instead of str(e)
    • or create the generated arg model with ConfigDict(hide_input_in_errors=True), then str(e) prints [type=string_type] without input_value=...

    Checked both on pydantic 2.13.5.

    It is worth checking the other validation paths too (structured output validation, resource template params). They probably format the error the same way.

    While looking at this, I found the same leak in the validation-error path of my own guard library, so this pattern is easy to miss.

  2. added a commit that references this issue on Sep 25, 2026
    2a6b2bc
  3. MohammadaminAlbooyeh commented on Sep 25, 2026

    @MohammadaminAlbooyeh

    I've implemented a fix and opened PR #3582. The approach: set hide_input_in_errors=True on ArgModelBase.model_config — a one-line change that suppresses input_value from all pydantic validation errors for tool arguments. All 59 existing tests pass. Happy to address the Copilot review comments (nested model coverage, ValidationError instead of Exception, end-to-end test) if a maintainer assigns me.

  4. liwenjie200543 commented on Sep 25, 2026

    @liwenjie200543

    Repro confirmed on main, plus one data point on the design.

    Where it leaks

    src/mcp/server/mcpserver/tools/base.py:150-153:

    except ValidationError as exc:
        # The caller's arguments don't match the input schema: the model's mistake
        # to read and correct, so it is reported like a deliberate ToolError.
        raise ToolError(f"Error executing tool {self.name}: {exc}") from exc

    str(ValidationError) includes input_value, so whatever the client sent comes back in the
    error result. With pydantic 2.13.5, a tool taking patient_id: int returns:

    1 validation error for Args
    patient_id
      Input should be a valid integer, unable to parse string as an integer
      [type=int_parsing, input_value='MRN-8837-Jane-Doe-DOB-1974-03-02', input_type=str]
        For further information visit https://errors.pydantic.dev/2.13/v/int_parsing
    

    Why the validation path is the odd one out

    The SDK already treats "do not echo the original back" as a rule rather than a nicety.
    UnexpectedToolError (src/mcp/server/mcpserver/exceptions.py:61-72) documents that its message
    is only Error executing tool <name> "so nothing from the original reaches the client", while
    __cause__ keeps the detail for the server log. run()'s own docstring repeats it: "A crash
    does not, so nothing from an unexpected exception reaches the client." Argument validation is the
    one branch that forwards pydantic's full text.

    The option I would take

    Dropping input_value by default is the right call. The open question is whether it needs a new
    public setting, and I would rather not add one. The repository's own guidance is to prefer
    maintainability and consistency over new capabilities, and UnexpectedToolError already shows the
    shape to copy: a short message to the client, the detail kept server-side on __cause__. Applying
    that here -- sanitised by default, original exception preserved for the server log, no new
    configuration surface -- extends a rule the SDK already follows instead of introducing a switch
    that has to be maintained.

    hide_input_in_errors=True on ArgModelBase.model_config is one line and covers the common case.
    The reason I would not stop there is nested models: a field whose type is itself a BaseModel
    carries its own model config, so a nested validation error can still render the input value.
    Worth confirming against whichever approach is chosen.

    Disclosure: AI-assisted. The pydantic snippet above was run locally against 2.13.5, and both
    files were read at the line numbers quoted.

  5. MohammadaminAlbooyeh commented on Sep 25, 2026

    @MohammadaminAlbooyeh

    I've updated the branch with fixes for all three Copilot review comments:

    1. Nested model coverage (High) — switched from relying solely on hide_input_in_errors=True on ArgModelBase to formatting the error via exc.errors(include_input=False) in Tool.run(). This guarantees nested user-defined BaseModel fields are also sanitized, regardless of their own config.

    2. End-to-end test (Medium) — added test_validation_error_does_not_echo_input_via_tool_run which drives the full stack through Tool.run() and asserts the sensitive value is absent from the ToolError message.

    3. ValidationError vs Exception (Low) — already addressed in the previous commit.

    All 61 tests pass. Branch: MohammadaminAlbooyeh:fix/hide-input-in-validation-errors

  6. MohammadaminAlbooyeh commented on Sep 25, 2026

    @MohammadaminAlbooyeh

    Can I be assigned to this issue? I've already implemented a fix on my fork.

  7. liwenjie200543 commented on Sep 25, 2026

    @liwenjie200543

    Correction to my comment above — I got the nested-model point wrong, and I checked it properly
    before anyone builds on it.

    I wrote that a nested BaseModel field "carries its own model config, so a nested validation
    error can still render the input value". That is backwards. Measured on pydantic 2.13.5, with the
    flag set on the outer model only, i.e. the one handed to model_validate:

    shape input_value in the final message
    flat field absent
    nested BaseModel absent
    list[Model], dict[str, Model] absent
    Optional[Model], Union[Model, int] absent
    list[list[Model]], dict[str, list[Model]] absent
    nested field_validator raising absent
    three levels deep absent

    Two controls show the flag is what does the work: the same shapes with no flag set do echo
    input_value, and setting it on the nested model only — leaving the outer model at its default —
    still echoes. The flag is read from the model you call model_validate() on and applies to
    everything below it. A nested model's own setting is not consulted: one that sets the flag to
    True while the outer model leaves it at the default still echoes the value.

    So ConfigDict(hide_input_in_errors=True) on the generated arguments model does cover
    def fn(payload: Payload): there is no nested gap, and the two approaches in the thread are
    equivalent in coverage. What is left is shape rather than safety — one line on the model config,
    versus formatting where the message is built with errors(include_input=False). I have no strong
    preference between them; the second does make the masking visible at the point it happens.

    One thing neither switch covers, worth noting once: both remove the input_value / input_type
    that pydantic attaches. Neither can remove a value that a field or model validator wrote into its
    own exception message. Different problem, same feature area.

    Disclosure: AI-assisted. Every row above was run locally against pydantic 2.13.5.

  8. MohammadaminAlbooyeh commented on Sep 26, 2026

    @MohammadaminAlbooyeh

    Thanks @liwenjie200543 for the thorough verification. Good to know the flag on the outer model covers all nested shapes with no gap — that clears up the original concern that prompted the switch to exc.errors(include_input=False).

    Both approaches are now confirmed equivalent in coverage. I'm happy to revert to the one-liner (ConfigDict(hide_input_in_errors=True) on ArgModelBase) for a smaller diff if maintainers prefer that shape. Just let me know which direction to go.

  9. added
    enhancementRequest for a new feature that's not currently supported
    on Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementRequest for a new feature that's not currently supported

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions