Skip to content

deps: add ata JSON Schema validator for node.config.json#62603

Draft
mertcanaltin wants to merge 6 commits intonodejs:mainfrom
mertcanaltin:feat/json-schema-validation
Draft

deps: add ata JSON Schema validator for node.config.json#62603
mertcanaltin wants to merge 6 commits intonodejs:mainfrom
mertcanaltin:feat/json-schema-validation

Conversation

@mertcanaltin
Copy link
Copy Markdown
Member

@mertcanaltin mertcanaltin commented Apr 5, 2026

Adds ata-validator (v0.8.0) to validate node.config.json against its JSON Schema before parsing.

Clear error messages for invalid config instead of generic "Invalid value" errors.

Example output:

Invalid configuration in node.config.json: /nodeOptions/import: expected exactly one oneOf match, got 0

What's included:

  • deps/ata/ with ata-validator built using ATA_NO_RE2 (no extra dependencies, only simdjson which is already in core)
  • Schema validation step in node_config_file.cc before existing parsing
  • process.versions.ata

Spec compliance:

  • 94.2% Draft 2020-12 test suite (1156/1227)
  • 95.3% @exodus/schemasafe test suite
  • $dynamicRef / $anchor: 42/42 (100%)

Security:

  • Recursion depth guard for circular $ref
  • __proto__ safe const/enum comparison
  • Date format range validation
  • OSS-Fuzz submitted, 283/283 JSONTestSuite passing

refs: #62598

Add ata-validator (v0.4.3) as a new dependency for JSON Schema
validation. Built with ATA_NO_RE2 to avoid RE2/abseil dependencies.
Uses simdjson (already in core) for JSON parsing.

Refs: nodejs#62598
Validate the configuration file against its JSON Schema using
ata-validator as a supplementary check after the existing parser.
This provides a safety net for type errors the parser might miss.

Refs: nodejs#62598
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 5, 2026
@mertcanaltin mertcanaltin marked this pull request as draft April 5, 2026 20:03
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 5, 2026

Codecov Report

❌ Patch coverage is 30.00000% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.70%. Comparing base (12249cc) to head (c28adff).

Files with missing lines Patch % Lines
src/node_config_file.cc 26.31% 10 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62603      +/-   ##
==========================================
- Coverage   91.53%   89.70%   -1.83%     
==========================================
  Files         352      695     +343     
  Lines      147833   214544   +66711     
  Branches    23148    41083   +17935     
==========================================
+ Hits       135321   192466   +57145     
- Misses      12255    14122    +1867     
- Partials      257     7956    +7699     
Files with missing lines Coverage Δ
src/node_metadata.cc 92.85% <100.00%> (ø)
src/node_config_file.cc 76.71% <26.31%> (ø)

... and 461 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@juanarbol
Copy link
Copy Markdown
Member

IMHO this is a bit too much, an extra dep for a schema parser inside Node.js (not to mention that is not even v1.x)

This is not a blocker, this is just my personal opinion

@mertcanaltin
Copy link
Copy Markdown
Member Author

That's a fair concern. The dependency is intentionally minimal, it only uses simdjson which is already in core (no new external dependencies since it's built with ATA_NO_RE2).

On the version, you're right it's pre-1.0, but it already passes 96.9% of the official JSON Schema Test Suite. Happy to work toward 1.0 if that's a requirement.

@TheOneTheOnlyJJ
Copy link
Copy Markdown
Contributor

Maybe ata could also be exposed to JS userland as an API if it ends up being vendored in?

@mertcanaltin
Copy link
Copy Markdown
Member Author

Yes, that's the plan! The initial PR focuses on node.config.json validation as a starting point, but exposing it as a public API is the natural next step. We discussed this in #62598, Joyee mentioned it would be the strongest use case.

The main prerequisite is getting the security story solid first, we've already submitted to OSS-Fuzz and all 283 nst/JSONTestSuite tests are passing.

@ChALkeR
Copy link
Copy Markdown
Member

ChALkeR commented Apr 7, 2026

@mertcanaltin check also https://www.npmjs.com/package/@exodus/schemasafe testsuite

@mertcanaltin
Copy link
Copy Markdown
Member Author

Thanks @ChALkeR, I ran the schemasafe test suite against ata (draft2020-12):

95.3% pass rate, 1103/1157, 59 skipped (remote refs).

Main gaps are unevaluatedProperties/Items annotation tracking, some $ref URI edge cases (URN, absolute-path), grapheme counting in minLength/maxLength, and a few optional format semantics.

Running the suite also helped me catch a few things I fixed since: recursion depth guard for circular $ref, proto handling in const/enum, and date format range checks.

$dynamicRef is fully passing now (42/42). Working on the rest.

Notable changes:
- $anchor, $dynamicAnchor, $dynamicRef support (42/42 spec tests)
- Recursion depth guard for circular $ref (prevents stack overflow)
- __proto__ key handling in const/enum comparison
- Date format month/day range validation
- 94.2% Draft 2020-12 spec compliance (1156/1227 tests)
- 95.3% @exodus/schemasafe test suite pass rate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants