Omit parsed JSON values from JsonParseError messages

[Amaury Chamayou (achamayou)]

Follow-up identified during the final security review of #8354.

Problem

The READ_REQUIRED_* macros in include/ccf/ds/json.h built the missing-required-field error as "Missing required field '<name>' in object: " + j.dump(), and the from_json_required_fields type check used "Expected object, found: " + j.dump(). For a private JWK passed to ccf.crypto.jwkToPem() with a missing field (e.g. qi), the exception what() therefore contained d, p, q, dp, dq in full. That text becomes the JS InternalError message and, when the debug-only log_exception_details / return_exception_details options are enabled, reaches node logs or HTTP responses. The exception storage and dump() temporaries are also outside the reach of the scrubbing guards added in #8354.

Changes

• include/ccf/ds/json.h: drop j.dump() from both messages. They are now Missing required field '<name>' in object and Expected object, with no detail about the parsed value. The field path is still available via JsonParseError::pointer() / describe().
• src/ds/test/json_schema.cpp: regression tests asserting that values are absent from both messages, for the plain and _WITH_RENAMES macro paths.
• CHANGELOG.md: entry under 7.0.16 Fixed.

No JS-visible result or error type changes; only the exception message text differs.

Validation

• json_schema unit test compiled directly with clang (header-only target) and run: 15 test cases, 160 assertions pass.
• clang-format --dry-run --Werror on both C++ files: clean. Prettier on CHANGELOG.md: clean.
• scripts/ci-checks.sh could not be run locally (Windows CRLF checkout breaks the bash scripts and the worktree .git pointer under WSL); CI is required before merge.

[Copilot]

🔵 Needs a closer look

Unresolved findings remain around key escaping, renamed type-check coverage, test assertions, and changelog scope.

Pull request overview

Updates JSON parse errors to omit parsed values, reducing sensitive data exposure.

Changes:

• Reports JSON keys or type names instead of serialized values.
• Adds regression tests for standard and renamed macros.
• Documents the fix in the changelog.

File summaries

File	Changes
src/ds/test/json_schema.cpp	Adds regression coverage for sanitized errors.
include/ccf/ds/json.h	Removes parsed values from JSON diagnostics.
CHANGELOG.md	Documents the security fix.

Review details

Suppressed comments (4)

CHANGELOG.md:21

• This release note is broader than the implementation: src/node/receipt.cpp still puts j.dump() into JsonParseError messages for missing required receipt fields and non-object receipt values. Either update those remaining paths or qualify this entry as applying to the JSON-schema macro-generated messages changed here.

- `ccf::JsonParseError` messages for a missing required field or a non-object value no longer include a serialisation of the parsed JSON, which could contain sensitive values such as private JWK fields. They now list the field names present, or the JSON type found (#8363).

include/ccf/ds/json.h:55

• Object member names are untrusted JSON input, but this appends them verbatim. A key containing a newline or another control character will therefore enter JsonParseError::what() and be logged as raw text when log_exception_details is enabled, allowing log-line injection; the old j.dump() escaped these characters. Escape/JSON-quote each key before joining while still omitting values.

      keys.push_back(item.key());

include/ccf/ds/json.h:855

• The _WITH_RENAMES macro has a separate non-object type-check branch here, but the new tests only exercise the renamed missing-field branch. A regression in this branch could reintroduce j.dump() for scalar input while the suite still passes; add a renamed-type scalar-input assertion that reports the JSON type and excludes SECRET_VALUE.

      throw ccf::JsonParseError( \
        std::string("Expected object, found: ") + j.type_name()); \

src/ds/test/json_schema.cpp:62

• These substring checks do not verify that the helper actually lists b and c: both letters occur in the surrounding word object, so an empty field list would still pass. Assert the emitted list (for example, fields: [b, c]) so this regression test covers the new behavior.

      REQUIRE(msg.find("b") != std::string::npos);
      REQUIRE(msg.find("c") != std::string::npos);

• Files reviewed: 3/3 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The renamed-field macro’s changed non-object error path lacks the claimed regression coverage.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced

[Amaury Chamayou (achamayou)]

This release note is broader than the implementation

Qualified the changelog entry in 91fdc76 to cover only JSON-schema-macro-generated messages. The hand-written receipt parsing errors are unchanged by this PR.

Object member names are untrusted JSON input

The earlier key-escaping and field-list-assertion findings concern code removed in e9a5fb0: these errors no longer list input keys or types, and the corresponding field-list assertions are gone. No key-list escaping helper is needed with the current generic messages.